This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Set visibility option to hidden #21924
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
am11
changed the title
Member
Author
|
@jkotas, I have gotten to the point where PAL tests are passing, but managed assemblies are failing in binder during the load. Here is a sample trace from lldb: Setup cd coreclr
./build.sh x64 checked
./build-test.sh x64 checked
./build-test.sh x64 checked generatelayoutonly
bash bin/tests/Linux.x64.Checked/JIT/CheckProjects/CheckProjects/CheckProjects.sh \
-coreroot=`pwd`/bin/Product/Linux.x64.Checked -debug=`command -v lldb`
# inside lldb REPL
(lldb) break set -E C++
(lldb) r
(lldb) btTrace * thread #1, name = 'corerun', stop reason = breakpoint 1.1
* frame #0: 0x00007ffff76bbc30 libstdc++.so.6`__cxa_throw
frame #1: 0x00007ffff618d3f5 libcoreclr.so`EEFileLoadException::Throw(path=u"System.Runtime, Version=4.2.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a", hr=-2147024894, pInnerException=<unavailable>) at clrex.cpp:2005
frame #2: 0x00007ffff636b709 libcoreclr.so`AssemblySpec::Bind(AppDomain*, int, CoreBindResult*, int, int) [inlined] ThrowLoadError(pSpec=0x00007fffffffcba0) at coreassemblyspec.cpp:100
frame #3: 0x00007ffff636b6c0 libcoreclr.so`AssemblySpec::Bind(this=0x00007fffffffcba0, pAppDomain=<unavailable>, fThrowOnFileNotFound=YES, pResult=0x00007fffffffc6e0, fNgenExplicitBind=NO, fExplicitBindToNativeImage=NO) at coreassemblyspec.cpp:195
frame #4: 0x00007ffff63070e9 libcoreclr.so`AppDomain::BindAssemblySpec(this=0x0000000000658900, pSpec=0x00007fffffffcba0, fThrowOnFileNotFound=YES, fUseHostBinderIfAvailable=<unavailable>) at appdomain.cpp:6177
frame #5: 0x00007ffff60f71dc libcoreclr.so`PEFile::LoadAssembly(this=0x00000000006a2480, kAssemblyRef=587202561, pImport=0x00000000006b7c40, szWinRtTypeNamespace=0x0000000000000000, szWinRtTypeClassName=0x0000000000000000) at pefile.cpp:1592
frame #6: 0x00007ffff632fbb1 libcoreclr.so`Module::LoadAssembly(this=<unavailable>, pDomain=0x0000000000658900, kAssemblyRef=587202561, szWinRtTypeNamespace=0x0000000000000000, szWinRtTypeClassName=0x0000000000000000) at ceeload.cpp:5318
frame #7: 0x00007ffff6316f4f libcoreclr.so`Assembly::FindModuleByTypeRef(pModule=<unavailable>, tkType=<unavailable>, loadFlag=Load, pfNoResolutionScope=NO) at assembly.cpp:1149
frame #8: 0x00007ffff634c915 libcoreclr.so`ClassLoader::LoadTypeDefOrRefThrowing(pModule=0x00007fff7d7d60e8, typeDefOrRef=16777221, fNotFoundAction=ThrowIfNotFound, fUninstantiated=FailIfUninstDefOrRef, tokenNotToLoad=0, level=CLASS_LOAD_APPROXPARENTS) at clsload.cpp:2778
frame #9: 0x00007ffff634d148 libcoreclr.so`ClassLoader::LoadApproxTypeThrowing(pModule=0x00007fff7d7d60e8, tok=<unavailable>, pSigInst=0x00007fffffffd220, pClassTypeContext=<unavailable>) at clsload.cpp:3183
frame #10: 0x00007ffff634d5b2 libcoreclr.so`ClassLoader::LoadApproxParentThrowing(pModule=0x00007fff7d7d60e8, cl=33554434, pParentInst=0x00007fffffffd220, pClassTypeContext=0x00007fffffffd1d0) at clsload.cpp:3237
frame #11: 0x00007ffff622a086 libcoreclr.so`ClassLoader::CreateTypeHandleForTypeDefThrowing(pModule=<unavailable>, cl=<unavailable>, inst=Instantiation @ 0x00007fffffffd480, pamTracker=0x00007fffffffd500) at methodtablebuilder.cpp:11931
frame #12: 0x00007ffff634df65 libcoreclr.so`ClassLoader::CreateTypeHandleForTypeKey(pKey=0x00007fffffffd840, pamTracker=0x00007fffffffd500) at clsload.cpp:3364
frame #13: 0x00007ffff634d89b libcoreclr.so`ClassLoader::DoIncrementalLoad(pTypeKey=0x00007fffffffd840, typeHnd=TypeHandle @ 0x00007fffffffd4f8, currentLevel=<unavailable>) at clsload.cpp:3292
frame #14: 0x00007ffff634f32f libcoreclr.so`ClassLoader::LoadTypeHandleForTypeKey_Body(this=0x00000000006a4f80, pTypeKey=0x00007fffffffd840, typeHnd=<unavailable>, targetLevel=<unavailable>) at clsload.cpp:4074
frame #15: 0x00007ffff6349b18 libcoreclr.so`ClassLoader::LoadTypeHandleForTypeKey(this=0x00000000006a4f80, pTypeKey=0x00007fffffffd840, typeHnd=<unavailable>, targetLevel=CLASS_LOADED, pInstContext=0x0000000000000000) at clsload.cpp:3793
frame #16: 0x00007ffff634b79b libcoreclr.so`ClassLoader::LoadTypeDefThrowing(pModule=<unavailable>, typeDef=33554434, fNotFoundAction=ThrowIfNotFound, fUninstantiated=FailIfUninstDefOrRef, tokenNotToLoad=<unavailable>, level=CLASS_LOADED, pTargetInstantiation=0x0000000000000000) at clsload.cpp:2662
frame #17: 0x00007ffff634ca80 libcoreclr.so`ClassLoader::LoadTypeDefOrRefThrowing(pModule=0x00007fff7d7d60e8, typeDefOrRef=33554434, fNotFoundAction=ThrowIfNotFound, fUninstantiated=FailIfUninstDefOrRef, tokenNotToLoad=0, level=CLASS_LOADED) at clsload.cpp:2840
frame #18: 0x00007ffff631960e libcoreclr.so`Assembly::GetEntryPoint(this=0x000000000062e050) at assembly.cpp:1811
frame #19: 0x00007ffff6319067 libcoreclr.so`Assembly::ExecuteMainMethod(this=0x000000000062e050, stringArgs=0x00007fffffffdf58, waitForOtherThreads=YES) at assembly.cpp:1689
frame #20: 0x00007ffff6050837 libcoreclr.so`CorHost2::ExecuteAssembly(this=<unavailable>, dwAppDomainId=<unavailable>, pwzAssemblyPath=<unavailable>, argc=<unavailable>, argv=0x0000000000000000, pReturnValue=0x00007fffffffe10c) at corhost.cpp:478
frame #21: 0x00007ffff6018017 libcoreclr.so`::coreclr_execute_assembly(hostHandle=<unavailable>, domainId=<unavailable>, argc=<unavailable>, argv=<unavailable>, managedAssemblyPath=<unavailable>, exitCode=<unavailable>) at unixinterface.cpp:412
frame #22: 0x0000000000402cb3 corerun`ExecuteManagedAssembly(currentExeAbsolutePath="/home/vagrant/coreclr/bin/Product/Linux.x64.Checked/corerun", clrFilesAbsolutePath=<unavailable>, managedAssemblyAbsolutePath="/home/vagrant/coreclr/bin/tests/Linux.x64.Checked/JIT/CheckProjects/CheckProjects/CheckProjects.exe", managedAssemblyArgc=0, managedAssemblyArgv=0x0000000000000000) at coreruncommon.cpp:429
frame #23: 0x0000000000401bde corerun`corerun(argc=<unavailable>, argv=<unavailable>) at corerun.cpp:149
frame #24: 0x00007ffff6ca3b97 libc.so.6`__libc_start_main(main=(corerun`main at corerun.cpp:161), argc=2, argv=0x00007fffffffe458, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007fffffffe448) at libc-start.c:310
frame #25: 0x000000000040180a corerun`_start + 42The innerException is empty at this point. Is there any interesting value I could check, or some place where a breakpoint can be set to spot name of the symbol it is looking for? |
Member
I do not know how this change can possibly lead to exception like this. Looks like some non-trivial interaction with the compiler. |
Member
janvorli
reviewed
Jan 17, 2019
Member
|
I can see test runs failing with the following assert with this change: |
Member
Author
|
The first exception in SOS' # entered lldb with:
# bash bin/tests/Linux.x64.Checked/JIT/CheckProjects/CheckProjects/CheckProjects.sh \
# -coreroot=`pwd`/bin/Product/Linux.x64.Checked -debug=`command -v lldb`
break set -E C++
plugin load ~/coreclr/bin/Product/Linux.x64.Checked/libsosplugin.so
run
# will break on first C++ exception
dumpstackdumpstack output |
Member
|
@am11 I've tried to build stuff with your change locally and it seems the issue is that System.Globalization.Native.so doesn't export any symbols (it failed on my box when trying to get GlobalizationNative_LoadICU export). So the setting of the visibility leaked into the build of this shared library too. Which is not bad per se, but we need to take care of marking the exposed APIs visibility in there too. |
am11
changed the title
Member
Author
|
@dotnet-bot, test Windows_NT x64 Checked Innerloop Build and Test please. |
Member
Author
|
@janvorli, thank you for pointing it out! Exporting symbols in System.Globalization.Native fixed the exception. 🎉 I have not redefined compiler option in globalization's cmake, as compileoptions.cmake are being inherited via CMakeLists.txt at the root of the repo. Is it by accident or design? If it is former, I will add that option in |
Member
|
The inheritance of build options is by design, I've just haven't realized we need to update the System.Globalization.Native exports too. |
Member
Author
I think it is already covered:
|
janvorli
reviewed
Jan 23, 2019
Member
Ah, great then! |
Member
Author
|
@dotnet-bot, test this please. |
Member
Author
|
(only five jobs get triggered after the merge, trying the remainder) |
Member
Author
Member
|
The only thing I keep thinking about is the fact that we manually mark (almost?) all of the PAL APIs as DLLEXPORT and so we had to touch so many files. I wonder if we could include the DLLEXPORT in the PALAPI macro and handle the few cases (IIRC you've mentioned there are some) in a special way - e.g. by creating PALAPI_NOEXPORT macro that would be the same as the original PALAPI. |
Member
Author
|
@janvorli, added |
Member
|
Could you please resolve conflicts / undo most the changes under |
Member
Author
|
Sure, I have rebased against master and removed the re-definition of visibility compiler option from |
Member
Author
|
@janvorli, thanks, there was a merge conflict, I have resolved it. Could this be merged? |
Member
|
@am11 yes, once the CI legs complete, I'll merge it. |
Member
|
@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test please |
|
This change produces desktop build errors: Could you please push a PR with the fix for that? |
Member
Author
|
@sandreenko, sorry about that. Is there any CI job configured for desktop build? I will try to reproduce it locally. |
Member
|
@am11 The "desktop build" is a special closed source build of .NET Framework with CoreCLR RyuJIT. There is no CI for it; not any way for you to do it. @sandreenko I think it would be best if you or somebody else who has the "desktop build" environment setup took care of fixing it. I guess the fix should be to define DLLEXPORT somewhere; but it is hard to tell what exactly will make the desktop build happy. |
Member
|
(It may need a fix in the closed source part of the desktop build.) |
mikedn
reviewed
Feb 9, 2019
| #ifndef FEATURE_MERGE_JIT_AND_ENGINE | ||
|
|
||
| extern "C" BOOL WINAPI DllMain(HANDLE hInstance, DWORD dwReason, LPVOID pvReserved) | ||
| extern "C" DLLEXPORT BOOL WINAPI DllMain(HANDLE hInstance, DWORD dwReason, LPVOID pvReserved) |
|
jit's CMake file generates a .def file on the file. Is that still needed? And coreclr's CMake does something similar. |
Member
|
It is still needed. It is a "post filter" that prunes the exported symbols. The symbols that we export now are symbols that need to be exported by libmscordac.so, libclrjit.so and libmscordi.so, but each of them should export only subset of those. |
|
Not sure I understand, are you referring to |
Member
|
libclrjit.so links to PAL on Unix, so by default, it would export all the functions that are exported by PAL. That's why we need to have the generated linker script that ensures that we expose only the three functions defined in it. The linker script on Linux prunes the list of exports to only the functions listed in the script. As for the DllMain, we still need to mark it as DLLEXPORT for Unix. I believe that Windows linker exports that automatically for dlls, so having the DLLEXPORT on it for Windows doesn't change anything. |
The |
Member
|
DLLEXPORT is defined to nothing on Windows, so DllMain is not actually getting exported. DllMain needs to be exported from clrjit on Unix, so that the PAL Win32 emulator can call it. We have been trimming the PAL over time and the DllMain emulation is one part that is a good candidate to trim, but we are not there yet. I have noticed other inconsistencies and DLLEXPORTs that should not be needed. Submitted WIP #22500 that should make this easier to understand. |
Merged
picenka21
pushed a commit
to picenka21/runtime
that referenced
this pull request
Feb 18, 2022
Commit migrated from dotnet/coreclr@5bb7eb6
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Defined and used
DllExportmacro instead ofPUB(aligned with the name used in CoreFX).While testing in different operating systems, I found that it is possible to have just
pyexecutable available in PATH via default installation withoutpython, so I have added it in probing paths inside cmake and sh files as well.Closes #2484