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.

Sign up for GitHub

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

[NativeAOT-LLVM] Add WasmExternalDwarf property to create an external_debug_info section #2786

Open
wants to merge 6 commits into
base: feature/NativeAOT-LLVM
Choose a base branch
from
Open

[NativeAOT-LLVM] Add WasmExternalDwarf property to create an external_debug_info section #2786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
804111f
add WasmExternalDwarf to create an external_debug_info section and pl…
yowl Nov 18, 2024
dd84ce5
Use StripSymbols which is true by default
yowl Nov 20, 2024
e8fd31f
move gseparate-dwarf to browser/emcc ItemGroup
yowl Nov 20, 2024
27e99ef
Update src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Nativ…
yowl Nov 21, 2024
b12b969
Update src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Nativ…
yowl Nov 21, 2024
5cffb73
feedback, drop DotNetJsApi condition
yowl Dec 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,10 @@

<Delete Files="$(PublishDir)\$(TargetName).wasm" Condition="'$(_targetOS)' == 'browser' and '$(DotNetJsApi)' != 'true'"/>
<Copy SourceFiles="$(NativeOutputPath)$(TargetName).wasm" DestinationFolder="$(PublishDir)" Condition="'$(_targetOS)' == 'browser' and '$(DotNetJsApi)' != 'true'"/>


<Delete Files="$(PublishDir)\$(TargetName).debug.wasm" Condition="'$(_targetOS)' == 'browser'"/>
<Copy SourceFiles="$(WasmSymbolFile)" DestinationFolder="$(PublishDir)" Condition="'$(_targetOS)' == 'browser' and Exists('$(WasmSymbolFile)')"/>

<ItemGroup>
<_FilesToCopyToNative Include="$(NativeOutputPath)\dotnet.native.js" />
<_FilesToCopyToNative Include="$(NativeOutputPath)\dotnet.native.wasm" />
Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ The .NET Foundation licenses this file to you under the MIT license.
<PropertyGroup Condition="'$(_targetOS)' == 'wasi'">
<WasmCompilerPath>&quot;$(WASI_SDK_PATH)/bin/clang++&quot;</WasmCompilerPath>
<WasmLinkerPath>&quot;$(WASI_SDK_PATH)/bin/clang&quot;</WasmLinkerPath>
</PropertyGroup>
</PropertyGroup>

<!-- Invoke the compilers in parallel. -->
<ItemGroup>
Expand Down Expand Up @@ -537,6 +537,7 @@ The .NET Foundation licenses this file to you under the MIT license.

<PropertyGroup>
<IlcIsLibLikeApp Condition="'$(IlcIsLibLikeApp)' == '' and ('$(NativeLib)' != '' or '$(CustomNativeMain)' != '')">true</IlcIsLibLikeApp>
<WasmSymbolFile>$([System.IO.Path]::ChangeExtension('$(NativeBinary)', '.debug.wasm'))</WasmSymbolFile>
</PropertyGroup>

<ItemGroup Condition="'$(InvariantGlobalization)' != 'true'">
Expand Down Expand Up @@ -587,6 +588,7 @@ The .NET Foundation licenses this file to you under the MIT license.
<CustomLinkerArg Include="-s TOTAL_STACK=$(IlcWasmStackSize)" />
<CustomLinkerArg Condition="'$(WasmEnableJSBigIntIntegration)' == 'true'" Include="-s WASM_BIGINT=1" />
<CustomLinkerArg Condition="'$(IlcLlvmExceptionHandlingModel)' == 'cpp'" Include="-s DISABLE_EXCEPTION_CATCHING=0" />
<CustomLinkerArg Condition="'$(StripSymbols)' == 'true'" Include="-gseparate-dwarf=&quot;$(WasmSymbolFile)&quot;" />
Copy link

@SingleAccretion SingleAccretion Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, does this embeds an absolute path, doesn't it. I think it would better to embed a relative path, i. e. just the file name. Otherwise I don't see it working even in our case where we copy from /native to /publish.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, testing what this property does, this in fact the correct way to use it. Using an absolute path results in the relative path, and using a relative path result in something that is relative to the current working directory. Tricky but workable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I follow the outcome of the comment here. It is ok as is? I think it was working when I tested it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is ok as is?

Yes.


<CustomLinkerArg Include="$(EmccFlags)" />
<CustomLinkerArg Include="-s MAXIMUM_MEMORY=$(EmccMaximumHeapSize)" Condition="'$(EmccMaximumHeapSize)' != ''" />
Expand Down