Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constructor size is twice as large on Clang17.0.2 with MTE enabled #69939

Closed
P1119r1m opened this issue Oct 23, 2023 · 6 comments · Fixed by #70186
Closed

Constructor size is twice as large on Clang17.0.2 with MTE enabled #69939

P1119r1m opened this issue Oct 23, 2023 · 6 comments · Fixed by #70186

Comments

@P1119r1m
Copy link

P1119r1m commented Oct 23, 2023

TL;DR

The bug is actually that Clang/ld.lld generates redundant executable code in the final binary.
This can be reproduced on (Clang > 17.0.0) when building C code for AARCH64 with the Memory Tagging Extension (MTE) compilation flags.
Example code simply stores function pointers in ".ctors" section.

Steps to reproduce the bug

To reproduce the problem 2 files should be created:

  • main.c

    typedef int (*ctor_t)(void);
    
    #define __constructor__    \
        __attribute__((__used__)) __attribute__((__section__(".ctors")))
    
    #define DECLARE_CONSTRUCTOR(fn)    \
        static ctor_t ctor_##fn __constructor__ = fn
    
    static int init_FIRST_func(void) { return 0; }
    static int init_SECOND_func(void) { return 0; }
    
    DECLARE_CONSTRUCTOR(init_FIRST_func);
    DECLARE_CONSTRUCTOR(init_SECOND_func);
    
    int main(int argc, char *argv[]) {
        (void)argc;
        (void)argv;
        return 0;
    }
    
  • build.sh

    #!/bin/bash
    
    ## CONFIG
    
    # 16.0.6 (llvmorg-v16.0.6 tag) - THE PROBLEM COULD NOT BE REPRODUCED
    CLANG_DIR=... !!! USE YOUR PATH !!! ...
    # 17.0.0 (llvmorg-v17-init tag) - THE PROBLEM COULD NOT BE REPRODUCED
    CLANG_DIR=... !!! USE YOUR PATH !!! ...
    # 17.0.2 (llvmorg-17.0.2 tag) - THE PROBLEM COULD BE REPRODUCED
    CLANG_DIR=... !!! USE YOUR PATH !!! ...
    # 17.0.3 (llvmorg-17.0.3 tag) - THE PROBLEM COULD BE REPRODUCED
    CLANG_DIR=... !!! USE YOUR PATH !!! ...
    
    CC="${CLANG_DIR}/bin/clang"
    LLD="${CLANG_DIR}/bin/ld.lld"
    OBJDUMP="${CLANG_DIR}/bin/llvm-objdump"
    
    CC_FLAGS="-march=armv8.3-a+memtag -fsanitize=memtag"  # COMMENT THIS CODE TO "FIX" THE ISSUE!
    
    ## COMPILE
    ${CC} ${CC_FLAGS} -c main.c -o main.o
    
    ## LINK
    ${LLD} main.o -o ./main.syms
    
    ## ANALYZE <BEGIN>
    
    echo -e "\n\n>>>>>>>> Show functions' addresses in compiled *.o file <<<<<<<<"
    ${OBJDUMP} -dS main.o | grep '<init_'
    
    echo -e "\n\n>>>>>>>> Show Clang version <<<<<<<<"
    ${CC} --version
    
    echo -e "\n\n>>>>>>>> Show contstructors' code in final binary <<<<<<<<"
    ${OBJDUMP} -dS -j ".ctors" main.syms
    
    ## ANALYZE <END>
    

The output for the "source build.sh" run command is following:

$ source build.sh
...
0000000000220230 <ctor_init_SECOND_func>:
  220230: f4 01 21 00   .word   0x002101f4    <---- NOTE: 16 bytes (and this is a problem, IMHO)
  220234: 00 00 00 00   .word   0x00000000
  220238: 00 00 00 00   .word   0x00000000
  22023c: 00 00 00 00   .word   0x00000000

In case of using "Clang17.0.0" from the "llvmorg-v17-init" tag:

0000000000220228 <ctor_init_SECOND_func>:
  220228: f4 01 21 00   .word   0x002101f4    <---- NOTE: 8 bytes
  22022c: 00 00 00 00   .word   0x00000000
@P1119r1m
Copy link
Author

@hctim

Dear Mitch Phillips,

I found that the issue mentioned in this issue was added between the following tags:

  • llvmorg-v16.0.6
  • llvmorg-v17.0.0-rc1

Also, I analyzed the MTE related activity for the commits between the mentioned tags and found that you are the author of them, for example:

Could you please analyze the current issue to see if it could be related to the changes you made?

Thank you!

@hctim
Copy link
Collaborator

hctim commented Oct 24, 2023

G'day Victor,

Yeah, this is almost certainly due to MTE globals instrumentation. We pad all global variables to be a multiple of 16 bytes, which includes your ctor_init_FIRST_func and ctor_init_SECOND_func fnptrs.

I'm guessing this breaks something in the dynamic loader as it's expecting the fnptrs in the .ctors section to be 16 bytes each?

I think explicitly declaring a constructor function with __attribute__((section(".ctors"))) is pretty niche. If you want to whack this yourself without much changes, you can add __attribute__((no_sanitize("memtag"))) to your DECLARE_CONSTRUCTOR macro. But I'm not sure why you wouldn't use the normal __attribute__((constructor(<x>))) macro instead?

@hctim hctim closed this as completed Oct 24, 2023
@hctim hctim reopened this Oct 24, 2023
@hctim
Copy link
Collaborator

hctim commented Oct 24, 2023

(misclick on the close-with-comment button)

@P1119r1m
Copy link
Author

P1119r1m commented Oct 24, 2023

Dear Mitch,

thank you very much for your quick reply!

I'm guessing this breaks something in the dynamic loader as it's expecting the fnptrs in the .ctors section to be 16 bytes each?

Unfortunately yes. We have a few ideas to solve the problem, but your response was very helpful.

If you want to whack this yourself without much changes, you can add attribute((no_sanitize("memtag"))) to your DECLARE_CONSTRUCTOR macro.

This really solved the problem:

#define __constructor__	\
	__attribute__((__used__)) __attribute__((__section__(".ctors"))) \
	__attribute__((no_sanitize("memtag")))

But I'm not sure why you wouldn't use the normal attribute((constructor())) macro instead?

Great advice!
Thanks again!
This will also solve the problem (UPD1: Re-checked. This isn't true if using with function pointers!):

#define DECLARE_CONSTRUCTOR(fn)	\
	static ctor_t ctor_##fn __attribute__((constructor)) __attribute__((__section__(".ctors"))) = fn
#endif

UPD2.
It is ok to use "attribute((constructor))" with functions (instead of function pointers):

#define __constructor__    \
    __attribute__((__used__)) \
    __attribute__((__section__(".ctors"))) \
    __attribute__((constructor)) \

static int __constructor__ init_FIRST_func(void) { return 0; }
static int __constructor__ init_SECOND_func(void) { return 0; }

int main(int argc, char *argv[]) {
    (void)argc;
    (void)argv;
    return 0;
}

However, in this case, function pointers should be accessed and called using ".init_array" section.

P.S.
Let me clarify, is the reported issue not a bug?
Should we close it?

@hctim
Copy link
Collaborator

hctim commented Oct 24, 2023

I think you're doing something very niche by declaring a constructor using __attribute__((sections(".ctors"))) :)

This breaks under all sanitizers that mess with global variables (asan, hwasan, mte).

Given this is the first time I'm hearing about it, and the examples I could find in a quick search (android, chrome, incl. third party libraries, etc.) were in binutils/clang/lld and not in user code, I don't think it's a huge priority to fix. But it does look cheap, let me take a quick hack at it.

hctim added a commit that referenced this issue Nov 1, 2023
Looks like there's code out there that, instead of using
'__attribute__((constructor(x)))' to add constructor functions, they
just declare a global function pointer and use
'__attribute__((section('.ctors')))' instead.

Problem is, with memtag-globals, we pad the global function pointer to
be 16 bytes large. This of course means we have an 8-byte real function
pointer, then 8 bytes of zero padding, and this trips up the loader when
it processes this section.

Fixes #69939
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 1, 2023

@llvm/issue-subscribers-backend-aarch64

Author: Victor Signaevskyi (P1119r1m)

### TL;DR The bug is actually that Clang/ld.lld generates redundant executable code in the final binary. This can be reproduced on (Clang > 17.0.0) when building C code for AARCH64 with the Memory Tagging Extension (MTE) compilation flags. Example code simply stores function pointers in ".ctors" section.

Steps to reproduce the bug

To reproduce the problem 2 files should be created:

  • main.c

    typedef int (*ctor_t)(void);
    
    #define __constructor__    \
        __attribute__((__used__)) __attribute__((__section__(".ctors")))
    
    #define DECLARE_CONSTRUCTOR(fn)    \
        static ctor_t ctor_##fn __constructor__ = fn
    
    static int init_FIRST_func(void) { return 0; }
    static int init_SECOND_func(void) { return 0; }
    
    DECLARE_CONSTRUCTOR(init_FIRST_func);
    DECLARE_CONSTRUCTOR(init_SECOND_func);
    
    int main(int argc, char *argv[]) {
        (void)argc;
        (void)argv;
        return 0;
    }
    
  • build.sh

    #!/bin/bash
    
    ## CONFIG
    
    # 16.0.6 (llvmorg-v16.0.6 tag) - THE PROBLEM COULD NOT BE REPRODUCED
    CLANG_DIR=... !!! USE YOUR PATH !!! ...
    # 17.0.0 (llvmorg-v17-init tag) - THE PROBLEM COULD NOT BE REPRODUCED
    CLANG_DIR=... !!! USE YOUR PATH !!! ...
    # 17.0.2 (llvmorg-17.0.2 tag) - THE PROBLEM COULD BE REPRODUCED
    CLANG_DIR=... !!! USE YOUR PATH !!! ...
    # 17.0.3 (llvmorg-17.0.3 tag) - THE PROBLEM COULD BE REPRODUCED
    CLANG_DIR=... !!! USE YOUR PATH !!! ...
    
    CC="${CLANG_DIR}/bin/clang"
    LLD="${CLANG_DIR}/bin/ld.lld"
    OBJDUMP="${CLANG_DIR}/bin/llvm-objdump"
    
    CC_FLAGS="-march=armv8.3-a+memtag -fsanitize=memtag"  # COMMENT THIS CODE TO "FIX" THE ISSUE!
    
    ## COMPILE
    ${CC} ${CC_FLAGS} -c main.c -o main.o
    
    ## LINK
    ${LLD} main.o -o ./main.syms
    
    ## ANALYZE &lt;BEGIN&gt;
    
    echo -e "\n\n&gt;&gt;&gt;&gt;&gt;&gt;&gt;&gt; Show functions' addresses in compiled *.o file &lt;&lt;&lt;&lt;&lt;&lt;&lt;&lt;"
    ${OBJDUMP} -dS main.o | grep '&lt;init_'
    
    echo -e "\n\n&gt;&gt;&gt;&gt;&gt;&gt;&gt;&gt; Show Clang version &lt;&lt;&lt;&lt;&lt;&lt;&lt;&lt;"
    ${CC} --version
    
    echo -e "\n\n&gt;&gt;&gt;&gt;&gt;&gt;&gt;&gt; Show contstructors' code in final binary &lt;&lt;&lt;&lt;&lt;&lt;&lt;&lt;"
    ${OBJDUMP} -dS -j ".ctors" main.syms
    
    ## ANALYZE &lt;END&gt;
    

The output for the "source build.sh" run command is following:

$ source build.sh
...
0000000000220230 &lt;ctor_init_SECOND_func&gt;:
  220230: f4 01 21 00   .word   0x002101f4    &lt;---- NOTE: 16 bytes (and this is a problem, IMHO)
  220234: 00 00 00 00   .word   0x00000000
  220238: 00 00 00 00   .word   0x00000000
  22023c: 00 00 00 00   .word   0x00000000

In case of using "Clang17.0.0" from the "llvmorg-v17-init" tag:

0000000000220228 &lt;ctor_init_SECOND_func&gt;:
  220228: f4 01 21 00   .word   0x002101f4    &lt;---- NOTE: 8 bytes
  22022c: 00 00 00 00   .word   0x00000000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants