labath requested changes to this revision.
labath added inline comments.
This revision now requires changes to proceed.


================
Comment at: lldb/source/Utility/CMakeLists.txt:29
+  PROPERTIES COMPILE_OPTIONS
+  "-fcxx-exceptions"
+)
----------------
kastiglione wrote:
> kastiglione wrote:
> > JDevlieghere wrote:
> > > kastiglione wrote:
> > > > the `std::regex` constructor throws `std::regex_error` if the pattern 
> > > > is invalid. For this reason, exceptions are enabled for this one file.
> > > What happens when exceptions are disabled? What does it mean to have this 
> > > enabled for a single file? I don't know if it's part of the LLVM 
> > > developer guide, but LLVM is supposed to build without RTTI and without 
> > > exceptions. 
> > > What happens when exceptions are disabled?
> > 
> > This cmake config enables exceptions for this one file, independent of 
> > `LLVM_ENABLE_EH`. No other source files will be allowed to catch or throw 
> > exceptions.
> > 
> > > What does it mean to have this enabled for a single file?
> > 
> > It means this file can compile with a try/catch, and that inside this file, 
> > exceptions can be caught.
> > 
> > > I don't know if it's part of the LLVM developer guide, but LLVM is 
> > > supposed to build without RTTI and without exceptions.
> > 
> > llvm has `LLVM_ENABLE_EH` which allows llvm to be built with exceptions 
> > support enabled. Similarly, `LLVM_ENABLE_RTTI` allows RTTI to be enabled. 
> > It seems that both are disabled as a default, but not as a hard requirement.
> > 
> > I wondered if enabling RTTI would needed for exceptions, but at least for 
> > this code, the answer is no. The `catch (const std::regex_error &e)` block 
> > is exercised by `TestBreakpointRegexError.py`, so we know this code can and 
> > does catch an exception of that type, and accesses the error's member 
> > functions.
> > 
> > 
> What makes me believe this use of exceptions is ok is that the API boundary 
> continues to be exception free, callers continue to be exception disabled. 
> Only the internal implementation, knows about exceptions.
That is definitely not ok. ODR madness awaits therein. The standard library is 
full of functions that effectively do
```
#ifdef EXCEPTIONS
  something();
#else
  something_else();
#endif
```
Compiling just one file with exceptions enabled can cause two different 
versions of those functions to appear. It is sufficient that this file 
instantiates one function whose behavior is dependent on exceptions being 
available. When linking, the linker has to choose one of the two versions, and 
there's no telling which one will it use. This means that exceptions can creep 
in into the supposedly exception-free code, or vice-versa.

The only way this would be safe is if this code was in a shared library, and we 
took care to restrict the visibility of all symbols except the exported api of 
that library. And I don't think that's worth it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132307/new/

https://reviews.llvm.org/D132307

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to