Don't use _ASSERTE in runtime tests#68299
Conversation
In debug configurations, use of assert() (aka _ASSERTE) requires the PAL_frpintf function Fixes dotnet#68279
|
Tagging subscribers to this area: @tommcdon Issue DetailsIn debug configurations, use of Fixes #68279
|
lambdageek
added
area-PAL-coreclr
|
Do we have a CI leg that will build the runtime tests in the debug configuration? |
trylek
left a comment
trylek
left a comment
There was a problem hiding this comment.
LGTM but I'd welcome confirmation from JanV who's the PAL expert.
|
AFAIK we only do checked configurations in the lab as debug is prohibitively slow. |
lambdageek
added
the
NO-MERGE
That macro requires linking the coreclrpal static library, which the runtime tests don't currently do.
|
I'm going to try to change this PR so that the profiler tests don't depend on the PAL. |
Maybe we should do a debug run with Mono - we don't have as noticeable a difference, I think (nearly all of our asserts are on in Release, too). /cc @steveisok - maybe a Linux x64 debug build that runs the runtime tests (also debug) in the rolling builds? |
lambdageek
changed the title
lambdageek
removed
the
NO-MERGE
|
|
||
| # Add this subdir. We install the headers for the jit. | ||
| add_subdirectory(../coreclr/pal/prebuilt/inc ../coreclr/pal/prebuilt/inc) | ||
| add_subdirectory(${CLR_REPO_ROOT_DIR}/src/coreclr/pal/prebuilt/inc ../coreclr/pal/prebuilt/inc) |
There was a problem hiding this comment.
We could use: ${CLR_ARTIFACTS_OBJ_DIR}/coreclr/pal/prebuilt/inc as we have used in other places for abs. path.
All tests were green without the second arg, so it's probably just copy pasted and actually not needed by anything.
There was a problem hiding this comment.
Leaving out the binary dir is an error:
CMake Error at CMakeLists.txt:15 (add_subdirectory):
add_subdirectory not given a binary directory but the given source
directory
"/home/aleksey/working/dotnet-runtime/src/coreclr/pal/prebuilt/inc" is not
a subdirectory of "/home/aleksey/working/dotnet-runtime/src/tests". When
specifying an out-of-tree source a binary directory must be explicitly
specified.
Using ${CLR_ARTIFACTS_OBJ_DIR}/coreclr/pal/prebuilt/inc works, except I'm not actually convinced it's the right thing. the normal coreclr cmake stuff goes to artifacts/obj/coreclr/<<Linux.x64.Debug>>/pal/prebuilt/inc. so we're making another copy. In which case it actually feels more correct to just drop it into a subdirectory of the runtime tests' binary dir. somehting like add_subdirectory (${CLR_REPO_ROOT_DIR}/src/coreclr/pal/prebuilt/inc coreclrpal)
There was a problem hiding this comment.
Yup, short paths would be nice. Some files have name clashes (canonical example is: config.h which comes from multiple places), I think that's why maintaining the same hierarchy under obj is used for uniqueness.
ghost
locked as resolved and limited conversation to collaborators
5 participants
In debug configurations, use of
_ASSERTErequires thePAL_frpintffunction and linking with thecoreclrpalstatic library.Instead just use
assert()from<assert.h>Fixes #68279