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

Android/AArch64 betterC putting out unreferenced __init symbol #2876

Closed
joakim-noah opened this issue Oct 15, 2018 · 10 comments
Closed

Android/AArch64 betterC putting out unreferenced __init symbol #2876

joakim-noah opened this issue Oct 15, 2018 · 10 comments

Comments

@joakim-noah
Copy link
Contributor

joakim-noah commented Oct 15, 2018

This is likely a long-standing dmd bug, but it's consistently reproducible with this Android/AArch64 example that a user recently reported on the forum. I was able to reproduce with the following sample code and the latest 1.12 release cross-compiling for Android from linux/x64:

extern(C) void main() {
    import core.stdc.stdio, core.sys.posix.sys.types;
    pthread_attr_t groo;
    printf("flags is %d\n", groo.flags);
}

Compiling with ./ldc2-1.12.0-linux-x86_64/bin/ldc2 -betterC -mtriple=aarch64-none-linux-android -c foo.d, I see the following symbol emitted:

$ readelf -sW foo.o| grep pthread
     8: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND _D4core3sys5posixQk5types14pthread_attr_t6__initZ

Compile with -mtriple=armv7-none-linux-android and the symbol's gone. I see it dumped in the IR too, so this is likely coming from the DMD frontend.

Linking this file on Android/AArch64 fails, because betterC doesn't link against druntime and so can't find the symbol. JacobKagamin's void-initialization workaround mentioned in the linked forum thread avoids the problem.

@kinke
Copy link
Member

kinke commented Oct 15, 2018

Why would that be a bug? The init symbol is emitted into the object file containing the struct/class declaration (i.e., druntime's core.sys.posix.sys.types), and as it's not zero-initialized, it's needed when allocating an instance without = void (blit from init symbol). I know people just expect everything to work with -betterC, but those familiar with its implementation know that it's worseD. Using templates and C forward declarations from Phobos/druntime likely works, everything else probably doesn't.

@kinke
Copy link
Member

kinke commented Oct 15, 2018

Btw the reason it's not zero-initialized (and thus only failing for Android/AArch64) is the chars array. Chars are initialized with 0xFF IIRC, so by initializing it with zeros (or using ubyte[16], void[16] etc.), it should be fine.

@joakim-noah
Copy link
Contributor Author

This is an extern(C) forward declaration, so it should work?

@kinke
Copy link
Member

kinke commented Oct 15, 2018

I'm not sure what a fwd-declared struct would be (an opaque declaration struct S;, or a full declaration with all functions just fwd declared, which still defines the initializer?) - but that's just pettifoggery ;), I was referring to functions.
It fails because of the chars array, as mentioned. Initializing the array explicitly with zeros isn't enough after a quick IR check (that may be considered a bug), but switching to ubyte or void makes the initialization a memset-to-0 instead of a blit-from-init-symbol.

@joakim-noah
Copy link
Contributor Author

I would have thought all such default initialization is turned off for extern(C) definitions. Guess ubyte is the better type here.

@kinke
Copy link
Member

kinke commented Oct 15, 2018

To be consistent with the other structs/unions a few lines down (and C char), it should probably be signed byte[16].

@PetarKirov
Copy link
Contributor

PetarKirov commented Oct 16, 2018

@joakim-noah since only functions and global variables have linkage, extern (C) affects only them. It has no effect on function definitions except for the calling convention and mangled name (i.e. it doesn't disable use of T.init for local variables), so it can't help in this situation.

@kinke semantically void[16] has the advantage of being unusable without casting. On the other hand the GC may scan such arrays for pointers:

S_ubyte(int, ubyte[16])
T.init: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
__traits(isZeroInit, S): true
__traits(getPointerBitmap, S): [20, 0]

S_void(int, void[16])
T.init: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
__traits(isZeroInit, S): true
__traits(getPointerBitmap, S): [20, 7]
struct S_ubyte { int x;  ubyte[16] y; }
struct S_void { int x; void[16] y; }

void main()
{
    import std.experimental.all;
    static foreach (S; AliasSeq!(S_ubyte, S_void))
    {{
        S s;
        writeln(S.stringof, Fields!S.stringof);
        writeln("T.init: ", (cast(ubyte*)&s)[0 .. S.sizeof]);
        writeln("__traits(isZeroInit, S): ", __traits(isZeroInit, S));
        writeln("__traits(getPointerBitmap, S): ", __traits(getPointerBitmap, S));
        writeln();
    }}
}

(Compile with DMD git master)

@kinke
Copy link
Member

kinke commented Oct 16, 2018

Thx for the reminder about the GC scanning difference, which makes sense.

@anon17
Copy link

anon17 commented Oct 26, 2018

https://issues.dlang.org/show_bug.cgi?id=17968 probably related

@kinke
Copy link
Member

kinke commented Mar 12, 2019

This specific issue (and quite a bunch of similar structs with previously default-initialized char buffers) has been fixed with v2.085, thanks to dlang/druntime@c322ec4 (I've verified it).

@kinke kinke closed this as completed Mar 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants