This is an archive of the discontinued LLVM Phabricator instance.

[libFuzzer][MSVC] Disable exceptions in MSVC headers
ClosedPublic

Authored by metzman on Jan 23 2019, 2:29 PM.

Details

Event Timeline

metzman created this revision.Jan 23 2019, 2:29 PM
metzman retitled this revision from [libFuzzer][MSVC} Disable exceptions in MSVC headers to [libFuzzer][MSVC] Disable exceptions in MSVC headers.Jan 23 2019, 2:33 PM
metzman edited the summary of this revision. (Show Details)Jan 23 2019, 3:07 PM
metzman added reviewers: rnk, morehouse.
metzman added subscribers: morehouse, rnk.

@rnk could you please take a look?

@morehouse are you OK with this change?

rnk added a comment.Jan 23 2019, 3:22 PM

Hm, I can't find fno-exceptions anywhere in the libfuzzer cmake files, so I'm questioning my original assumption that this code is supposed to have exceptions disabled. If libfuzzer is supposed to have exceptions enabled, we should leave /EHsc in place as is.

In D57119#1368624, @rnk wrote:

Hm, I can't find fno-exceptions anywhere in the libfuzzer cmake files, so I'm questioning my original assumption that this code is supposed to have exceptions disabled. If libfuzzer is supposed to have exceptions enabled, we should leave /EHsc in place as is.

Maybe Matt knows the answer to this?
AFAIK though, libFuzzer doesn't use exceptions.
Would there be a reason to allow exceptions even though one isn't using them?

morehouse accepted this revision.Jan 23 2019, 3:50 PM

libFuzzer doesn't disable exceptions itself, but the private libc++ it builds has exceptions disabled. So this should be fine for MSVC STL.

This revision is now accepted and ready to land.Jan 23 2019, 3:50 PM
metzman accepted this revision.Jan 24 2019, 5:06 PM

@rnk does this look good to land to you?

rnk accepted this revision.Jan 24 2019, 5:07 PM

lgtm

This revision was automatically updated to reflect the committed changes.
Herald added subscribers: Restricted Project, llvm-commits. · View Herald TranscriptJan 24 2019, 5:10 PM

I'm a bit curious about the root causes of this patch - is it for builds with MS STL or libc++, and what were the actual issues it was trying to fix? I'm getting mixed signals from the comments in the discussion thread above.

As things stand right now, the -D_HAS_EXCEPTIONS=0 option breaks libc++ when it runs on top of vcruntime, see D103947 for a fix for that and for discussion whether this is a setup that libc++ wants to commit to supporting.