[llvm-dlltool] Implement the --no-leading-underscore option

[mstorsjo]

Summary

This requires being able to opt out from adding the leading underscores
in COFFModuleDefinition. Normally it is added automatically for I386
type targets. We could either move the decision entirely to all
callers, letting the caller check the machine type and decide whether
underscores should be added, or keep the logic mostly as is, but allowing
opting out from the behaviour on I386.

I went with keeping the interface as is for now, but I'm open for
discussion on the matter.

This goes on top of D152360, for people wanting to cherry pick it.

[mati865]

llvm/lib/Object/COFFModuleDefinition.cpp

143

I know nothing about ARM on Windows so double-check it but shouldn't this also check for ARM?

[jeremyd2019]

FYI, binutils' dlltool also has a variant of that flag going the other way (--leading-underscores), causing underscores to be added on arches other than i386 (the default for leading underscores is "only on i386" but the flag can either set it to true or false explicitly). I don't see the utility of that, but then again I didn't really see the utility of --no-leading-underscores either until rust was trying to use it.

[mstorsjo]

llvm/lib/Object/COFFModuleDefinition.cpp

143

Nope, out of the current 4 active architectures, only i386 uses such prefixes. And I don’t think the other ones (alpha, mips, sh4, ppc, ia64) used any prefix either, but I don’t know for sure about them.

In most cases, i386 is the weirdo - https://devblogs.microsoft.com/oldnewthing/20040914-00/?p=37873

[mstorsjo]

In D152363#4403514, @jeremyd2019 wrote:

FYI, binutils' dlltool also has a variant of that flag going the other way (--leading-underscores), causing underscores to be added on arches other than i386 (the default for leading underscores is "only on i386" but the flag can either set it to true or false explicitly). I don't see the utility of that, but then again I didn't really see the utility of --no-leading-underscores either until rust was trying to use it.

Indeed; I didn’t see much need for adding that either. After thinking about it, I do see the need for this flag though, making things more straightforward, bypassing all the extra adding/removing of decorations.

llvm/lib/Object/COFFModuleDefinition.cpp

143

Oh, and this matches the existing machine checks that get folded in here, which check specifically for i386.

[jeremyd2019]

I applied D152359 D152360 D152361 and this to llvm 16.0.4, and then used that to successfully bootstrap rust on MSYS2's CLANG32 environment, which previously failed due to the unknown --temp-prefix argument, and even when that argument was dropped by a wrapper around llvm-dlltool, failed with linker errors due to --no-leading-underscore not being implemented.

llvm/include/llvm/Object/COFFModuleDefinition.h

struct COFFModuleDefinition {
   uint32_t MajorImageVersion = 0;
   uint32_t MinorImageVersion = 0;
   uint32_t MajorOSVersion = 0;
   uint32_t MinorOSVersion = 0;
 };
 
 Expected<COFFModuleDefinition>
 parseCOFFModuleDefinition(MemoryBufferRef MB, COFF::MachineTypes Machine,
-                          bool MingwDef = false);
+                          bool MingwDef = false, bool AddUnderscores = true);
 
 } // End namespace object.
 } // End namespace llvm.
 
 #endif

llvm/lib/Object/COFFModuleDefinition.cpp

public:
   }
 
 private:
   StringRef Buf;
 };
 
 class Parser {
 public:
-  explicit Parser(StringRef S, MachineTypes M, bool B)
-      : Lex(S), Machine(M), MingwDef(B) {}
+  explicit Parser(StringRef S, MachineTypes M, bool B, bool AU)
+      : Lex(S), Machine(M), MingwDef(B), AddUnderscores(AU) {
+    if (Machine != IMAGE_FILE_MACHINE_I386)
+      AddUnderscores = false;
+  }
 
   Expected<COFFModuleDefinition> parse() {
     do {
       if (Error Err = parseOne())
         return std::move(Err);
     } while (Tok.K != Eof);
     return Info;
   }
if (Tok.K == Equal) {
       if (Tok.K != Identifier)
         return createError("identifier expected, but got " + Tok.Value);
       E.ExtName = E.Name;
       E.Name = std::string(Tok.Value);
     } else {
       unget();
     }
 
-    if (Machine == IMAGE_FILE_MACHINE_I386) {
+    if (AddUnderscores) {
       if (!isDecorated(E.Name, MingwDef))
         E.Name = (std::string("_").append(E.Name));
       if (!E.ExtName.empty() && !isDecorated(E.ExtName, MingwDef))
         E.ExtName = (std::string("_").append(E.ExtName));
     }
 
     for (;;) {
       read();
for (;;) {
       }
       if (Tok.K == KwPrivate) {
         E.Private = true;
         continue;
       }
       if (Tok.K == EqualEqual) {
         read();
         E.AliasTarget = std::string(Tok.Value);
-        if (Machine == IMAGE_FILE_MACHINE_I386 && !isDecorated(E.AliasTarget, MingwDef))
+        if (AddUnderscores && !isDecorated(E.AliasTarget, MingwDef))
           E.AliasTarget = std::string("_").append(E.AliasTarget);
         continue;
       }
       unget();
       Info.Exports.push_back(E);
       return Error::success();
     }
   }
private:
   }
 
   Lexer Lex;
   Token Tok;
   std::vector<Token> Stack;
   MachineTypes Machine;
   COFFModuleDefinition Info;
   bool MingwDef;
+  bool AddUnderscores;
 };
 
 Expected<COFFModuleDefinition> parseCOFFModuleDefinition(MemoryBufferRef MB,
                                                          MachineTypes Machine,
-                                                         bool MingwDef) {
-  return Parser(MB.getBuffer(), Machine, MingwDef).parse();
+                                                         bool MingwDef,
+                                                         bool AddUnderscores) {
+  return Parser(MB.getBuffer(), Machine, MingwDef, AddUnderscores).parse();
 }
 
 } // namespace object
 } // namespace llvm

llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp

int llvm::dlltoolDriverMain(llvm::ArrayRef<const char *> ArgsArr) {
   if (auto *Arg = Args.getLastArg(OPT_m))
     Machine = getEmulation(Arg->getValue());
 
   if (Machine == IMAGE_FILE_MACHINE_UNKNOWN) {
     llvm::errs() << "unknown target\n";
     return 1;
   }
 
-  Expected<COFFModuleDefinition> Def =
-      parseCOFFModuleDefinition(*MB, Machine, /*MingwDef=*/true);
+  bool AddUnderscores = !Args.hasArg(OPT_no_leading_underscore);
+  Expected<COFFModuleDefinition> Def = parseCOFFModuleDefinition(
+      *MB, Machine, /*MingwDef=*/true, AddUnderscores);
 
   if (!Def) {
     llvm::errs() << "error parsing definition\n"
                  << errorToErrorCode(Def.takeError()).message();
     return 1;
   }
 
   // Do this after the parser because parseCOFFModuleDefinition sets OutputFile.

llvm/lib/ToolDrivers/llvm-dlltool/Options.td

 def D_long : JoinedOrSeparate<["--"], "dllname">, Alias<D>;
 
 def d: JoinedOrSeparate<["-"], "d">, HelpText<"Input .def File">;
 def d_long : JoinedOrSeparate<["--"], "input-def">, Alias<d>;
 
 def k: Flag<["-"], "k">, HelpText<"Kill @n Symbol from export">;
 def k_alias: Flag<["--"], "kill-at">, Alias<k>;
 
+def no_leading_underscore: Flag<["--"], "no-leading-underscore">,
+    HelpText<"Don't add leading underscores on symbols">;
+
 //==============================================================================
 // The flags below do nothing. They are defined only for dlltool compatibility.
 //==============================================================================
 
 def S: JoinedOrSeparate<["-"], "S">, HelpText<"Assembler">;
 def S_alias: JoinedOrSeparate<["--"], "as">, Alias<S>;
 
 def f: JoinedOrSeparate<["-"], "f">, HelpText<"Assembler Flags">;
 def f_alias: JoinedOrSeparate<["--"], "as-flags">, Alias<f>;
 
 def t: JoinedOrSeparate<["-"], "t">, HelpText<"Prefix for temporary files (ignored)">;
 def t_alias: JoinedOrSeparate<["--"], "temp-prefix">, Alias<t>;

llvm/test/tools/llvm-dlltool/no-leading-underscore.def

+; RUN: llvm-dlltool -k -m i386 --input-def %s --output-lib %t.a --no-leading-underscore --kill-at
+; RUN: llvm-readobj %t.a | FileCheck %s
+; RUN: llvm-nm %t.a | FileCheck %s -check-prefix=CHECK-NM
+
+LIBRARY test.dll
+EXPORTS
+func
+alias == func
+DecoratedFunction@4
+
+; CHECK:      Name type: name
+; CHECK-NEXT: Symbol: __imp_func
+; CHECK-NEXT: Symbol: func
+; CHECK:      Name type: undecorate
+; CHECK-NEXT: Symbol: __imp_DecoratedFunction@4
+; CHECK-NEXT: Symbol: DecoratedFunction@4
+
+; CHECK-NM: W alias
+; CHECK-NM: U func