The Wayback Machine - https://web.archive.org/web/20190322195341/https://github.com/dotnet/coreclr/pull/18873
Skip to content

/coreclr

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

Add prefix to DAC's PAL exports for alpine #18873

Merged
merged 1 commit into from Jul 13, 2018

Conversation

Labels
area-Diagnostics
5 participants
@mikem8361 @nbilling @omajid @noahfalk @chuckries
@mikem8361
Copy link
Member

mikem8361 commented Jul 11, 2018

Added some cmake logic to create assembly include mapping files. One that maps the
prefixed name (DAC_foo) to the actual name (foo) which is included in the DAC module
and another that maps the actual name to the prefixed name that is included in the SOS,
DBI and createdump modules.

The data exports like IID_IUnknown are not prefixed and don't need to be (immutable static data).

There were some C++ exports functions exported with their decorated names in
the CatchHardwareExceptionHolder and NativeExceptionHolderBase classes. Created
PAL_* style export functions that implements the code.

Fix lldb plugin cmake file to use LLDB_H/LLDB_LIB env vars to build it.

@mikem8361 mikem8361 added the area-Diagnostics label Jul 11, 2018

@mikem8361 mikem8361 added this to the 2.2 milestone Jul 11, 2018

@mikem8361 mikem8361 self-assigned this Jul 11, 2018

@mikem8361 mikem8361 requested review from chuckries and noahfalk Jul 11, 2018

@nbilling

This comment has been minimized.

Sign in to view
Copy link

nbilling commented Jul 11, 2018

<--- Nelson's account.
@chuckries

chuckries approved these changes Jul 12, 2018
View changes

@mikem8361

This comment has been minimized.

Sign in to view
Copy link
Member Author

mikem8361 commented Jul 12, 2018

@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test
@mikem8361
Add prefix to DAC's PAL exports for alpine 
Added some cmake logic to create assembly include mapping files. One that maps the
prefixed name (DAC_foo) to the actual name (foo) which is included in the DAC module
and another that maps the actual name to the prefixed name that is included in the SOS,
DBI and createdump modules.

The data exports like IID_IUnknown are not prefixed and don't need to be (immutable static data).

There were some C++ exports functions exported with their decorated names in
the CatchHardwareExceptionHolder and NativeExceptionHolderBase classes. Created
PAL_* style export functions that implements the code.

Fix lldb plugin cmake file to use LLDB_H/LLDB_LIB env vars to build it.
Loading status checks…
a620b50

@mikem8361 mikem8361 force-pushed the mikem8361:alpinepal branch from e569e23 to a620b50 Jul 13, 2018

@noahfalk

noahfalk reviewed Jul 13, 2018
View changes

src/pal/inc/pal.h
CatchHardwareExceptionHolder();
CatchHardwareExceptionHolder()
{
PAL_CatchHardwareExceptionHolderEnter();

This comment has been minimized.

Sign in to view
Copy link
@noahfalk

noahfalk Jul 13, 2018

Member

Does moving these implementations into the header relate to the renaming or is this something else? (not implying you need to move it, just trying to understand)

This comment has been minimized.

Sign in to view
Copy link
@mikem8361

mikem8361 Jul 13, 2018

Author Member

We were exporting the C++ decorated names for the constructor/destructor and I didn't want to depend on the compiler's name decoration format anymore.
@noahfalk

noahfalk approved these changes Jul 13, 2018
View changes

Copy link
Member

noahfalk left a comment

LGTM (albeit I'm not an expert by any means in cmake but it seemed reasonable)

@mikem8361 mikem8361 merged commit b89e230 into dotnet:master Jul 13, 2018
23 checks passed

23 checks passed

CROSS Check Build finished.
Details
CentOS7.1 x64 Checked Innerloop Build and Test Build finished.
Details
CentOS7.1 x64 Debug Innerloop Build Build finished.
Details
Linux-musl x64 Debug Build Build finished.
Details
OSX10.12 x64 Checked CoreFX Tests Build finished.
Details
OSX10.12 x64 Checked Innerloop Build and Test Build finished.
Details
Tizen armel Cross Checked Innerloop Build and Test Build finished.
Details
Ubuntu arm Cross Checked Innerloop Build and Test Build finished.
Details
Ubuntu arm64 Cross Debug Innerloop Build Build finished.
Details
Ubuntu x64 Checked CoreFX Tests Build finished.
Details
Ubuntu x64 Checked Innerloop Build and Test Build finished.
Details
Ubuntu x64 Formatting Build finished.
Details
WIP ready for review
Details
Windows_NT x64 Checked CoreFX Tests Build finished.
Details
Windows_NT x64 Checked Innerloop Build and Test Build finished.
Details
Windows_NT x64 Formatting Build finished.
Details
Windows_NT x64 full_opt ryujit CoreCLR Perf Tests Correctness Build finished.
Details
Windows_NT x64 min_opt ryujit CoreCLR Perf Tests Correctness Build finished.
Details
Windows_NT x86 Checked Innerloop Build and Test Build finished.
Details
Windows_NT x86 Release Innerloop Build and Test Build finished.
Details
Windows_NT x86 full_opt ryujit CoreCLR Perf Tests Correctness Build finished.
Details
Windows_NT x86 min_opt ryujit CoreCLR Perf Tests Correctness Build finished.
Details
license/cla All CLA requirements met.
Details

@mikem8361 mikem8361 deleted the mikem8361:alpinepal branch Jul 13, 2018

mikem8361 added a commit to mikem8361/coreclr that referenced this pull request Jul 13, 2018

@mikem8361
Add prefix to DAC's PAL exports for alpine (dotnet#18873) 
Added some cmake logic to create assembly include mapping files. One that maps the
prefixed name (DAC_foo) to the actual name (foo) which is included in the DAC module
and another that maps the actual name to the prefixed name that is included in the SOS,
DBI and createdump modules.

The data exports like IID_IUnknown are not prefixed and don't need to be (immutable static data).

There were some C++ exports functions exported with their decorated names in
the CatchHardwareExceptionHolder and NativeExceptionHolderBase classes. Created
PAL_* style export functions that implements the code.

Fix lldb plugin cmake file to use LLDB_H/LLDB_LIB env vars to build it.
84160ca

@mikem8361 mikem8361 referenced this pull request Jul 13, 2018

Closed

Alpine Linux coreclr, dac, dbi not honoring PAL symbol versions #18307

mikem8361 added a commit to mikem8361/coreclr that referenced this pull request Jul 16, 2018

@mikem8361
Add prefix to DAC's PAL exports for alpine (dotnet#18873) 
Added some cmake logic to create assembly include mapping files. One that maps the
prefixed name (DAC_foo) to the actual name (foo) which is included in the DAC module
and another that maps the actual name to the prefixed name that is included in the SOS,
DBI and createdump modules.

The data exports like IID_IUnknown are not prefixed and don't need to be (immutable static data).

There were some C++ exports functions exported with their decorated names in
the CatchHardwareExceptionHolder and NativeExceptionHolderBase classes. Created
PAL_* style export functions that implements the code.

Fix lldb plugin cmake file to use LLDB_H/LLDB_LIB env vars to build it.
b700cca

mikem8361 added a commit to mikem8361/coreclr that referenced this pull request Jul 26, 2018

@mikem8361
Add prefix to DAC's PAL exports for alpine (dotnet#18873) 
Added some cmake logic to create assembly include mapping files. One that maps the
prefixed name (DAC_foo) to the actual name (foo) which is included in the DAC module
and another that maps the actual name to the prefixed name that is included in the SOS,
DBI and createdump modules.

The data exports like IID_IUnknown are not prefixed and don't need to be (immutable static data).

There were some C++ exports functions exported with their decorated names in
the CatchHardwareExceptionHolder and NativeExceptionHolderBase classes. Created
PAL_* style export functions that implements the code.

Fix lldb plugin cmake file to use LLDB_H/LLDB_LIB env vars to build it.
f8e50f4

mikem8361 added a commit to mikem8361/coreclr that referenced this pull request Jul 30, 2018

@mikem8361
Add prefix to DAC's PAL exports for alpine (dotnet#18873) 
Added some cmake logic to create assembly include mapping files. One that maps the
prefixed name (DAC_foo) to the actual name (foo) which is included in the DAC module
and another that maps the actual name to the prefixed name that is included in the SOS,
DBI and createdump modules.

The data exports like IID_IUnknown are not prefixed and don't need to be (immutable static data).

There were some C++ exports functions exported with their decorated names in
the CatchHardwareExceptionHolder and NativeExceptionHolderBase classes. Created
PAL_* style export functions that implements the code.

Fix lldb plugin cmake file to use LLDB_H/LLDB_LIB env vars to build it.
43a022c

mikem8361 added a commit to mikem8361/coreclr that referenced this pull request Aug 3, 2018

@mikem8361
Add prefix to DAC's PAL exports for alpine (dotnet#18873) 
Added some cmake logic to create assembly include mapping files. One that maps the
prefixed name (DAC_foo) to the actual name (foo) which is included in the DAC module
and another that maps the actual name to the prefixed name that is included in the SOS,
DBI and createdump modules.

The data exports like IID_IUnknown are not prefixed and don't need to be (immutable static data).

There were some C++ exports functions exported with their decorated names in
the CatchHardwareExceptionHolder and NativeExceptionHolderBase classes. Created
PAL_* style export functions that implements the code.

Fix lldb plugin cmake file to use LLDB_H/LLDB_LIB env vars to build it.
7a28c53

mikem8361 added a commit to mikem8361/coreclr that referenced this pull request Aug 3, 2018

@mikem8361
Add prefix to DAC's PAL exports for alpine (dotnet#18873) 
Added some cmake logic to create assembly include mapping files. One that maps the
prefixed name (DAC_foo) to the actual name (foo) which is included in the DAC module
and another that maps the actual name to the prefixed name that is included in the SOS,
DBI and createdump modules.

The data exports like IID_IUnknown are not prefixed and don't need to be (immutable static data).

There were some C++ exports functions exported with their decorated names in
the CatchHardwareExceptionHolder and NativeExceptionHolderBase classes. Created
PAL_* style export functions that implements the code.

Fix lldb plugin cmake file to use LLDB_H/LLDB_LIB env vars to build it.
4da57b0

mikem8361 added a commit that referenced this pull request Aug 4, 2018

@mikem8361
Add prefix to DAC's PAL exports for alpine (#18873) (#19198) 
Added some cmake logic to create assembly include mapping files. One that maps the
prefixed name (DAC_foo) to the actual name (foo) which is included in the DAC module
and another that maps the actual name to the prefixed name that is included in the SOS,
DBI and createdump modules.

The data exports like IID_IUnknown are not prefixed and don't need to be (immutable static data).

There were some C++ exports functions exported with their decorated names in
the CatchHardwareExceptionHolder and NativeExceptionHolderBase classes. Created
PAL_* style export functions that implements the code.

Fix lldb plugin cmake file to use LLDB_H/LLDB_LIB env vars to build it.
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
eafbf02

@kevingosse kevingosse referenced this pull request Nov 13, 2018

Closed

Failure to get address of DAC_PAL_InitializeDLL on custom centos build #141

omajid added a commit to omajid/coreclr that referenced this pull request Nov 19, 2018

@omajid
Fix search for headers and libraries via LLVM_HOME 
Commit b89e230 (PR dotnet#18873) renamed WITH_LLDB_LIBS to LLDB_LIB_DIR and
WITH_LLDB_INCLUDES to LLDB_INCLUDE_DIR. But it forgot to update the
usage of these variables in some find_library and find_path calls.

As a result, setting LLVM_HOME is no longer enough to compile coreclr on
platforms with non-standard locations for lldb. This commit fixes that.
b555569

@omajid omajid referenced this pull request Nov 19, 2018

Merged

Fix search for headers and libraries via LLVM_HOME #21084

@omajid

This comment has been minimized.

Sign in to view
Copy link
Member

omajid commented Nov 19, 2018

This change introduced a regression when using LLVM_HOME to specify a non-standard directory for lldb headers. I opened a PR to fix it: #21084

janvorli added a commit that referenced this pull request Nov 20, 2018

@omajid @janvorli
Fix search for headers and libraries via LLVM_HOME (#21084) 
Commit b89e230 (PR #18873) renamed WITH_LLDB_LIBS to LLDB_LIB_DIR and
WITH_LLDB_INCLUDES to LLDB_INCLUDE_DIR. But it forgot to update the
usage of these variables in some find_library and find_path calls.

As a result, setting LLVM_HOME is no longer enough to compile coreclr on
platforms with non-standard locations for lldb. This commit fixes that.
38d3570
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.