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

Mangle conflict after ImportC statement expression gets expanded from macro #20831

Closed
ryuukk opened this issue Feb 6, 2025 · 6 comments
Closed
Assignees
Labels
Feature:ImportC Pertaining to ImportC support

Comments

@ryuukk
Copy link
Contributor

ryuukk commented Feb 6, 2025

I'm using ImportC in my project, and i'm compiling it as a shared library, the commit be8668e introduces the following issue:

/usr/bin/ld: warning: size of symbol `_D2it24nk_style_push_style_itemUPS3__C10nk_contextPSQs13nk_style_itemQsZ22__dgliteral_L19113_C28MFZv' changed from 56 in bin/game-lib.o to 58 in bin/game-lib.o
/usr/bin/ld: warning: NOTE: size discrepancies can cause real problems.  Investigation is advised.
/usr/bin/ld: warning: size of symbol `_D2it19nk_style_push_floatUPS3__C10nk_contextPffZ22__dgliteral_L19114_C28MFZv' changed from 56 in bin/game-lib.o to 58 in bin/game-lib.o
/usr/bin/ld: warning: NOTE: size discrepancies can cause real problems.  Investigation is advised.
/usr/bin/ld: warning: size of symbol `_D2it18nk_style_push_vec2UPS3__C10nk_contextPSQs7nk_vec2QlZ22__dgliteral_L19115_C28MFZv' changed from 56 in bin/game-lib.o to 58 in bin/game-lib.o
/usr/bin/ld: warning: NOTE: size discrepancies can cause real problems.  Investigation is advised.
/usr/bin/ld: warning: size of symbol `_D2it19nk_style_push_flagsUPS3__C10nk_contextPkkZ22__dgliteral_L19116_C28MFZv' changed from 56 in bin/game-lib.o to 58 in bin/game-lib.o
/usr/bin/ld: warning: NOTE: size discrepancies can cause real problems.  Investigation is advised.
/usr/bin/ld: warning: size of symbol `_D2it19nk_style_push_colorUPS3__C10nk_contextPSQs8nk_colorQmZ22__dgliteral_L19117_C28MFZv' changed from 56 in bin/game-lib.o to 58 in bin/game-lib.o
/usr/bin/ld: warning: NOTE: size discrepancies can cause real problems.  Investigation is advised.
/usr/bin/ld: warning: size of symbol `_D2it23nk_style_pop_style_itemUPS3__C10nk_contextZ22__dgliteral_L19119_C28MFZv' changed from 56 in bin/game-lib.o to 58 in bin/game-lib.o
/usr/bin/ld: warning: NOTE: size discrepancies can cause real problems.  Investigation is advised.
/usr/bin/ld: warning: size of symbol `_D2it18nk_style_pop_floatUPS3__C10nk_contextZ22__dgliteral_L19120_C28MFZv' changed from 56 in bin/game-lib.o to 58 in bin/game-lib.o
/usr/bin/ld: warning: NOTE: size discrepancies can cause real problems.  Investigation is advised.
/usr/bin/ld: warning: size of symbol `_D2it17nk_style_pop_vec2UPS3__C10nk_contextZ22__dgliteral_L19121_C28MFZv' changed from 56 in bin/game-lib.o to 58 in bin/game-lib.o
/usr/bin/ld: warning: NOTE: size discrepancies can cause real problems.  Investigation is advised.
/usr/bin/ld: warning: size of symbol `_D2it18nk_style_pop_flagsUPS3__C10nk_contextZ22__dgliteral_L19122_C28MFZv' changed from 56 in bin/game-lib.o to 58 in bin/game-lib.o
/usr/bin/ld: warning: NOTE: size discrepancies can cause real problems.  Investigation is advised.
/usr/bin/ld: warning: size of symbol `_D2it18nk_style_pop_colorUPS3__C10nk_contextZ22__dgliteral_L19123_C28MFZv' changed from 56 in bin/game-lib.o to 58 in bin/game-lib.o
/usr/bin/ld: warning: NOTE: size discrepancies can cause real problems.  Investigation is advised.

It's symbols from C source code: https://github.com/Immediate-Mode-UI/Nuklear/

It also causes heavy compilation slowdown

Build flags:

-shared -debug -betterC -fPIC

@ryuukk ryuukk changed the title Commit: be8668e9380d3cc07cffa183a77336ab09351f7f introduces linker problems with ImportC when building as a shared library Linker problems with ImportC when building as a shared library Feb 6, 2025
@ryuukk
Copy link
Contributor Author

ryuukk commented Feb 7, 2025

To reproduce::

  1. create 2 files:
    • app.d
    • bug.c

app.d

import bug;

bug.c

#define NK_IMPLEMENTATION
#include "nuklear.h"
  1. download the file: https://raw.githubusercontent.com/Immediate-Mode-UI/Nuklear/refs/heads/master/nuklear.h

  2. compile: dmd -betterC -shared app.d bug.c

ping @dkorpel since you are the author of the PR

and ping @WalterBright since it impacts ImportC

@thewilsonator thewilsonator added the Feature:ImportC Pertaining to ImportC support label Feb 7, 2025
@dkorpel
Copy link
Contributor

dkorpel commented Feb 9, 2025

I followed your steps, but I don't see those linker warnings or a slowdown. I tried dmd master, be8668e, and the commit before that. They all take ~0.094 seconds and don't output any warnings.

From inspecting nuklear.h and your linker output, I'm guessing there's a name collision when a macro containing an assert() gets expanded multiple times:

#define NK_STYLE_PUSH_IMPLEMENATION(prefix, type, stack) \
...
    NK_ASSERT(ctx);\
...

The GNU assert macro gets expanded to a statement expression, which ImportC translates to a function literal with a supposedly unique "dgliteral_LOCATION" name, but in your case different asserts get the same mangled name.

The location in your linker output point to lines 19113-19123, which doesn't match my copy of nuklear.h. Can you post the exact commit / content of your nuklear.h?

@ryuukk
Copy link
Contributor Author

ryuukk commented Feb 10, 2025

To make sure we are testing the same thing

OS: Linux x64
Linker: GNU ld (GNU Binutils) 2.44
LDC used to build DMD: 1.40.0:
Nuklear: master

I'm compiling a debug build of DMD this way: make install HOST_DMD=/usr/bin/ldmd2 ENABLE_RELEASE=0 ENABLE_LTO=0

EDIT:

Can you add -L=-v to the command to build the repro and paste information here please?

@dkorpel
Copy link
Contributor

dkorpel commented Feb 10, 2025

-L=-v gives:

GNU ld (GNU Binutils) 2.43.1
collect2 version 14.2.1 20250128
/usr/bin/ld -plugin /usr/lib/gcc/x86_64-pc-linux-gnu/14.2.1/liblto_plugin.so -plugin-opt=/usr/lib/gcc/x86_64-pc-linux-gnu/14.2.1/lto-wrapper -plugin-opt=-fresolution=/tmp/ccEUhLIz.res -plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lgcc_s -plugin-opt=-pass-through=-lc -plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lgcc_s --build-id --eh-frame-hdr --hash-style=gnu -m elf_x86_64 -shared -o .so /usr/lib/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../lib/crti.o /usr/lib/gcc/x86_64-pc-linux-gnu/14.2.1/crtbeginS.o -L/home/dennis/repos/dmd/generated/linux/debug/64/../../../../../phobos/generated/linux/debug/64 -L/usr/lib/gcc/x86_64-pc-linux-gnu/14.2.1 -L/usr/lib/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../lib -L/lib/../lib -L/usr/lib/../lib -L/usr/lib/gcc/x86_64-pc-linux-gnu/14.2.1/../../.. app.o -v --export-dynamic -lpthread -lm -lrt -ldl -lgcc --push-state --as-needed -lgcc_s --pop-state -lc -lgcc --push-state --as-needed -lgcc_s --pop-state /usr/lib/gcc/x86_64-pc-linux-gnu/14.2.1/crtendS.o /usr/lib/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../lib/crtn.o

Edit: I can reproduce it now. I'll try to reduce it.

@dkorpel
Copy link
Contributor

dkorpel commented Feb 10, 2025

Self-contained example:

#include "assert.h"

int* p;

#define IMPL() \
f()\
{\
    assert(p);\
    assert(4 < 3);\
}

void IMPL()

void main(void) {}

@dkorpel dkorpel changed the title Linker problems with ImportC when building as a shared library Mangle conflict after ImportC statement expression gets expanded from macro Feb 10, 2025
@dkorpel dkorpel self-assigned this Feb 10, 2025
dkorpel added a commit to dkorpel/dmd that referenced this issue Feb 11, 2025
dkorpel added a commit to dkorpel/dmd that referenced this issue Feb 11, 2025
@ryuukk
Copy link
Contributor Author

ryuukk commented Feb 13, 2025

Thanks for the fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ImportC Pertaining to ImportC support
Projects
None yet
Development

No branches or pull requests

3 participants