Skip to content

Re-enable SANITIZER_INTERCEPT_MALLOPT_AND_MALLINFO#14366

Merged
sbc100 merged 1 commit into
mainfrom
sanitizer_intercept_mallinfo
Jun 3, 2021
Merged

Re-enable SANITIZER_INTERCEPT_MALLOPT_AND_MALLINFO#14366
sbc100 merged 1 commit into
mainfrom
sanitizer_intercept_mallinfo

Conversation

@sbc100
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 commented Jun 3, 2021

The logic of this macro was inverted when the llvm-12 update to
compiler-rt (#14280). This caused asan.test_setjmp_noleak to start
failing.

Its not clear it we want to keep this long term but restoring
it at least fixes the failing test.

See #14360

The logic of this macro was inverted when the llvm-12 update to
compiler-rt (#14280).  This caused asan.test_setjmp_noleak to start
failing.

Its not clear it we want to keep this long term but restoring
it at least fixes the failing test.

See #14360
@sbc100 sbc100 requested a review from aheejin June 3, 2021 19:39
@aheejin
Copy link
Copy Markdown
Member

aheejin commented Jun 3, 2021

#14280's change for SANITIZER_INTERCEPT_MALLOPT_AND_MALLINFO is this, and it doesn't include SI_EMSCRIPTEN?
image

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Jun 3, 2021

#14280's change for SANITIZER_INTERCEPT_MALLOPT_AND_MALLINFO is this, and it doesn't include SI_EMSCRIPTEN?
image

But its defined as a negative conjunction in the old version.. so it ends up being true for emscripten.

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Jun 3, 2021

#14280's change for SANITIZER_INTERCEPT_MALLOPT_AND_MALLINFO is this, and it doesn't include SI_EMSCRIPTEN?
image

But its defined as a negative conjunction in the old version.. so it ends up being true for emscripten.

The condition on the left is true under emscripten right?

@aheejin
Copy link
Copy Markdown
Member

aheejin commented Jun 3, 2021

I think you're right... Then wouldn't it be good to reenable the failing test here too?

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Jun 3, 2021

I think you're right... Then wouldn't it be good to reenable the failing test here too?

The failing test is an asan test which was never disabled. Its currently red on the emscripten-releases waterfall.

Copy link
Copy Markdown
Member

@aheejin aheejin left a comment

Choose a reason for hiding this comment

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

Ah right, thanks.

@sbc100 sbc100 enabled auto-merge (squash) June 3, 2021 21:59
@sbc100 sbc100 disabled auto-merge June 3, 2021 22:18
@sbc100 sbc100 merged commit c95a28d into main Jun 3, 2021
@sbc100 sbc100 deleted the sanitizer_intercept_mallinfo branch June 3, 2021 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants