Skip to content

Conversation

@AlexGuteniev
Copy link
Contributor

@AlexGuteniev AlexGuteniev commented Jan 23, 2022

I don't think LIBRARYNAME is needed, although I may be missing something.

@BillyONeal indicated .src was introduced to make MSBuild and CMake output match.
Unfortunately, I cannot make MSBuild here, so I can't look into the issue.

But I want to solve this somehow simpler for a new satellite in #2502

I don't think LIBRARYNAME is needed
@AlexGuteniev AlexGuteniev requested a review from a team as a code owner January 23, 2022 10:44
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Jan 23, 2022
@BillyONeal
Copy link
Member

@BillyONeal indicated .src was introduced to make MSBuild and CMake output match.
Unfortunately, I cannot make MSBuild here, so I can't look into the issue.

Please compare dumpbin /EXPORTS before and after.

@StephanTLavavej StephanTLavavej added the info needed We need more info before working on this label Feb 2, 2022
@StephanTLavavej
Copy link
Member

Marking as info needed - knowing whether or not the export list has changed is necessary before merging this.

@AlexGuteniev
Copy link
Contributor Author

The diff of dumpbin /ALL shows only the case difference of the satellite DLL name, which I don't think is significant.
Moreover, other satellites have small name, not ALL CAPS name, so the new approach better matches with other satellites.

5c5
< Dump of file C:\Project\STL\out\build\x64_main\out\bin\amd64\msvcp140_atomic_wait_oss.dll
---
> Dump of file C:\Project\STL\out\build\x64\out\bin\amd64\msvcp140_atomic_wait_oss.dll
14c14
<         61FD80AF time date stamp Fri Feb  4 21:38:23 2022
---
>         61FD821D time date stamp Fri Feb  4 21:44:29 2022
40c40
<            1A74A checksum
---
>            11666 checksum
1182c1182
<   0000000180005980: 00 00 00 00 AF 80 FD 61 00 00 00 00 02 00 00 00  ....Ї.эa........
---
>   0000000180005980: 00 00 00 00 1D 82 FD 61 00 00 00 00 02 00 00 00  ......эa........
1184c1184
<   00000001800059A0: AF 80 FD 61 00 00 00 00 0D 00 00 00 DC 02 00 00  Ї.эa........Ь...
---
>   00000001800059A0: 1D 82 FD 61 00 00 00 00 0D 00 00 00 DC 02 00 00  ..эa........Ь...
1247,1248c1247,1248
<   0000000180005D90: C5 01 00 00 52 53 44 53 45 64 18 3E 84 DE 28 40  Е...RSDSEd.>.Ю(@
<   0000000180005DA0: BE FF 6E 9D 32 2D 8C 59 26 00 00 00 43 3A 5C 50  ѕяn.2-.Y&...C:\P
---
>   0000000180005D90: C5 01 00 00 52 53 44 53 24 91 2B FA C0 0E 94 47  Е...RSDS$.+ъА..G
>   0000000180005DA0: B5 20 85 7B 85 30 2B 92 01 00 00 00 43 3A 5C 50  µ .{.0+.....C:\P
1418,1419c1418,1419
<   0000000180006840: 1C 00 1D 00 1E 00 1F 00 4D 53 56 43 50 31 34 30  ........MSVCP140
<   0000000180006850: 5F 41 54 4F 4D 49 43 5F 57 41 49 54 5F 4F 53 53  _ATOMIC_WAIT_OSS
---
>   0000000180006840: 1C 00 1D 00 1E 00 1F 00 6D 73 76 63 70 31 34 30  ........msvcp140
>   0000000180006850: 5F 61 74 6F 6D 69 63 5F 77 61 69 74 5F 6F 73 73  _atomic_wait_oss
1633,1634c1633,1634
<     61FD80AF cv            60 00005D94     4B94    Format: RSDS, {3E186445-DE84-4028-BEFF-6E9D322D8C59}, 38, C:\Project\STL\out\build\x64\out\bin\amd64\msvcp140_atomic_wait_oss.pdb
<     61FD80AF coffgrp      2DC 00005DF4     4BF4    4C544347 (LTCG)
---
>     61FD821D cv            60 00005D94     4B94    Format: RSDS, {FA2B9124-0EC0-4794-B520-857B85302B92}, 1, C:\Project\STL\out\build\x64\out\bin\amd64\msvcp140_atomic_wait_oss.pdb
>     61FD821D coffgrp      2DC 00005DF4     4BF4    4C544347 (LTCG)
1636c1636
<   Section contains the following exports for MSVCP140_ATOMIC_WAIT_OSS.dll
---
>   Section contains the following exports for msvcp140_atomic_wait_oss.dll

@CaseyCarter CaseyCarter removed the info needed We need more info before working on this label Feb 4, 2022
@StephanTLavavej StephanTLavavej added build Related to the build system and removed enhancement Something can be improved labels Feb 4, 2022
@StephanTLavavej
Copy link
Member

This actually breaks the internal build with c1xx : fatal error C1356: unable to find mspdbcore.dll. (At first I thought it was unrelated, but it's directly caused by this change.)

I'm not sure how this is happening but we'll need to resolve this.

@StephanTLavavej StephanTLavavej self-requested a review February 6, 2022 04:05
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Feb 6, 2022

Comparing VS 2022 17.1 Preview 5's C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.31.31103\bin\Hostx86\x86\msvcp140_atomic_wait.dll against a freshly built D:\msvc\binaries\x86chk\bin\i386\msvcp140_atomic_wait.dll with these changes, I observe:

  • One of the dumpbin /all differences is (from VS to this PR):
    -4540 entry point (10004540)
    +4540 entry point (10004540) __DllMainCRTStartup@12
    I am not sure what this means.
  • dumpbin /exports displays a file name difference:
    -Section contains the following exports for MSVCP140_ATOMIC_WAIT.dll
    +Section contains the following exports for msvcp_atomic_wait.dll
    Observe that beyond the lowercase, the 140 is gone, even though it's present in the filename.
  • The exports themselves seem to be mangled differently.
    -1    0 00002830 __std_acquire_shared_mutex_for_instance
    +1    0 00002830 __std_acquire_shared_mutex_for_instance = ___std_acquire_shared_mutex_for_instance@4
    -2    1 00001D70 __std_atomic_compare_exchange_128
    +2    1 00001D70 __std_atomic_compare_exchange_128 = ___std_atomic_compare_exchange_128@24
    -3    2 00001DA0 __std_atomic_get_mutex
    +3    2 00001DA0 __std_atomic_get_mutex = ___std_atomic_get_mutex@4
    -4    3 00001DD0 __std_atomic_has_cmpxchg16b
    +4    3 00001DD0 __std_atomic_has_cmpxchg16b = ___std_atomic_has_cmpxchg16b@0

I observe no dumpbin /exports differences beyond lowercasing for the OSS build, as Alex reported, so this behavior is specific to the internal build. 😿

@StephanTLavavej
Copy link
Member

Exporting from a DLL Using DEF Files says:

A minimal DEF file must contain the following module-definition statements:

  • The first statement in the file must be the LIBRARY statement. This statement identifies the DEF file as belonging to a DLL. The LIBRARY statement is followed by the name of the DLL. The linker places this name in the DLL's import library.

@AlexGuteniev
Copy link
Contributor Author

  • The first statement in the file must be the LIBRARY statement

Still, LIBRARY says:

LIBRARY [library][BASE=address]

Remarks

The library argument specifies the name of the DLL. You can also use the /OUT linker option to specify the DLL's output name.

So it seems the name is optional, and we may use empty LIBRARY statement.

@StephanTLavavej
Copy link
Member

Closing as we discussed on Discord - we might investigate this later after overhauling the internal build to use CMake instead of MSBuild. Thanks again for looking into this!

@AlexGuteniev AlexGuteniev deleted the satellite_simplify branch February 6, 2022 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Related to the build system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants