Skip to content
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 the -fsanitize-blacklist option for the sanitizers. #2261

Merged
merged 7 commits into from
Aug 9, 2017

Conversation

JohanEngelen
Copy link
Member

Having fun with ASan :)
Blacklisting is necessary for some druntime functions, and LLVM/Clang already provides just the right functionality.

Will add the actual needed blacklist in a later PR.

@JohanEngelen
Copy link
Member Author

Ah ofcourse. Windows mangling.

@JohanEngelen
Copy link
Member Author

Any concerns?

@kinke
Copy link
Member

kinke commented Aug 7, 2017

Looks good after a quick glance, just one thing: we shouldn't use the IR function name, as the necessity to fix up the testcase for Win32 clearly shows. ;)

@JohanEngelen
Copy link
Member Author

I thought about using the "normal" function name, without the \01 added for Win32, but then I thought that because of different underscore conventions, the start of the mangled name is already different anyway so you'd have to use a * at the start already ("*_D...."). The blacklist for Clang is using mangled names, and I don't want to deviate from that.
What I can do is to shorten the StringRef by one if we detect a '\01' at the start of the string?

@JohanEngelen
Copy link
Member Author

(eager to get this in btw, it's annoying me with asan work)

@JohanEngelen
Copy link
Member Author

(also, I could get rid of the extra _ added for win32.......)

@kinke
Copy link
Member

kinke commented Aug 7, 2017

I meant just using the regular D mangle (mangleExact()), not the ugly stuff we have to add to make LLVM use that name for the object file [and without that dubious additional underscore on some platforms] in TargetABI::mangleFunctionForLLVM().

@JohanEngelen
Copy link
Member Author

Yep indeed, I am already reworking things. function.cpp should just check whether the FuncDecl is in the blacklist, without concerning itself about mangling/blacklist details. I've changed the lookup to use mangleExact (whose returned string is cached).

@JohanEngelen
Copy link
Member Author

(I wanted things to be as close as possible to the real mangled name in the object file, including hashing. But... I can think of other use cases where blacklisting using the non-hashed name makes more sense. So I'm happy with mangleExact too)

Copy link
Member

@kinke kinke left a comment

Choose a reason for hiding this comment

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

I took the liberty to address my last nit, passing front-end objects unnaturally by reference (and so wouldn't be callable from D via D object reference, which is to be mangled as pointer for C++ functions).

@JohanEngelen
Copy link
Member Author

The by-reference was intentional: the interface does not allow passing null pointers.
Inability of D code to bind to C++ class references leading to bad code :(

@JohanEngelen
Copy link
Member Author

I'll merge when Travis is green.

@kinke
Copy link
Member

kinke commented Aug 9, 2017

the interface does not allow passing null pointers.

Refs don't protect from passing nulls under the hood, the segfault (in func()) is just a FuncDeclaration *fd = nullptr; func(*fd) away.

@JohanEngelen JohanEngelen merged commit ec9ffe2 into ldc-developers:master Aug 9, 2017
@JohanEngelen JohanEngelen deleted the asanoptout branch August 9, 2017 21:21
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