This is an archive of the discontinued LLVM Phabricator instance.

[COFF, ARM64] Recognize the .hidden directive
Needs ReviewPublic

Authored by mgrang on Aug 3 2018, 5:04 PM.

Diff Detail

Event Timeline

mgrang created this revision.Aug 3 2018, 5:04 PM
pcc added a subscriber: pcc.Aug 3 2018, 5:06 PM

I don't think the .hidden directive is meaningful in COFF, is it?

mgrang added a comment.EditedAug 3 2018, 5:12 PM
In D50284#1188307, @pcc wrote:

I don't think the .hidden directive is meaningful in COFF, is it?

Yes, I don't think too. But we still need to compile hand-written assembly containing .hidden for COFF. That's why this patch simply eats up the .hidden directive without handling it in the MCWinCOFFStreamer.cpp. I did a small test and found that MSVS exports only those symbols explicitly marked as dllexport.

> cat a.c
__declspec( dllexport ) void foo();
void bar();
static void baz();
cl a.c
dumpbin /all a.obj

RAW DATA #1
  00000030: 45 58 50 4F 52 54 3A 66 6F 6F 20                 EXPORT:foo 

   Linker Directives
   -----------------
   /EXPORT:foo

I see that it only exported foo which was marked as dllexport, which means that by default it does not export any symbol. So I guess we don't explicitly need to handle the .hidden symbols.

Could someone please confirm if my understanding is correct?

In D50284#1188307, @pcc wrote:

I don't think the .hidden directive is meaningful in COFF, is it?

Yes, I don't think too. But we still need to compile hand-written assembly containing .hidden for COFF. That's why this patch simply eats up the .hidden directive without handling it in the MCWinCOFFStreamer.cpp. I did a small test and found that MSVS exports only those symbols explicitly marked as dllexport.

> cat a.c
__declspec( dllexport ) void foo();
void bar();
static void baz();
cl a.c
dumpbin /all a.obj

RAW DATA #1
  00000030: 45 58 50 4F 52 54 3A 66 6F 6F 20                 EXPORT:foo 

   Linker Directives
   -----------------
   /EXPORT:foo

I see that it only exported foo which was marked as dllexport, which means that by default it does not export any symbol. So I guess we don't explicitly need to handle the .hidden symbols.

Could someone please confirm if my understanding is correct?

This is correct - but I'm still hesitant to add this.

Also, in mingw, if no symbols are marked dllexport, all symbols are exported - and there's nothing there like .hidden to limit it. And GAS, which I'd consider the reference for handling directives in .s files, doesn't support .hidden for COFF.

Even though it's nice if handwritten assembly builds with as little modifications as possible, COFF/windows is a distinctly different target than ELF. Usually the assembly needs to handle some target specific differences wrt to function declarations etc, usually wrapped up in some sort of macro.

E.g. libunwind has got this in assembly.h:

#if defined(__APPLE__)
 
#define SYMBOL_IS_FUNC(name)
#define HIDDEN_SYMBOL(name) .private_extern name
#define NO_EXEC_STACK_DIRECTIVE
 
#elif defined(__ELF__)
 
#if defined(__arm__)
#define SYMBOL_IS_FUNC(name) .type name,%function
#else
#define SYMBOL_IS_FUNC(name) .type name,@function
#endif
#define HIDDEN_SYMBOL(name) .hidden name

#if defined(__GNU__) || defined(__FreeBSD__) || defined(__Fuchsia__) || \
    defined(__linux__)
#define NO_EXEC_STACK_DIRECTIVE .section .note.GNU-stack,"",%progbits
#else
#define NO_EXEC_STACK_DIRECTIVE
#endif
 
#elif defined(_WIN32)
 
#define SYMBOL_IS_FUNC(name)                                                   \
  .def name SEPARATOR                                                          \
    .scl 2 SEPARATOR                                                           \
    .type 32 SEPARATOR                                                         \
  .endef
#define HIDDEN_SYMBOL(name)
 
#define NO_EXEC_STACK_DIRECTIVE
 
#else
 
#error Unsupported target
 
#endif

So there, .hidden is handled like all other target specifics, and it isn't supported for macho anyway.

rnk added a comment.Aug 6 2018, 11:20 AM
In D50284#1188307, @pcc wrote:

I don't think the .hidden directive is meaningful in COFF, is it?

Yes, I don't think too. But we still need to compile hand-written assembly containing .hidden for COFF. That's why this patch simply eats up the .hidden directive without handling it in the MCWinCOFFStreamer.cpp. I did a small test and found that MSVS exports only those symbols explicitly marked as dllexport.

I don't see why we need to parse .hidden when targeting COFF, it isn't meaningful. Gas rejects it for win64:
t.s:3: Error: unknown pseudo-op: `.hidden'