[Lldb-commits] [lldb] r327356 - [ExpressionParser] Fix crash when evaluating invalid expresssions.

[Davide Italiano via lldb-commits]

On Tue, Mar 13, 2018 at 10:56 AM, Greg Clayton <clayborg at gmail.com> wrote:
>
>
> On Mar 13, 2018, at 10:25 AM, Davide Italiano via lldb-commits
> <lldb-commits at lists.llvm.org> wrote:
>
> On Tue, Mar 13, 2018 at 9:59 AM, Frédéric Riss via lldb-commits
> <lldb-commits at lists.llvm.org> wrote:
>
>
>
> On Mar 12, 2018, at 6:40 PM, Davide Italiano via lldb-commits
> <lldb-commits at lists.llvm.org> wrote:
>
> Author: davide
> Date: Mon Mar 12 18:40:00 2018
> New Revision: 327356
>
> URL: http://llvm.org/viewvc/llvm-project?rev=327356&view=rev
> Log:
> [ExpressionParser] Fix crash when evaluating invalid expresssions.
>
> Typical example, illformed comparisons (operator== where LHS and
> RHS are not compatible). If a symbol matched `operator==` in any
> of the object files lldb inserted a generic function declaration
> in the ASTContext on which Sema operates. Maintaining the AST
> context invariants is fairly tricky and sometimes resulted in
> crashes inside clang (or assertions hit).
>
> The real reason why this feature exists in the first place is
> that of allowing users to do something like:
> (lldb) call printf("patatino")
>
> even if the debug informations for printf() is not available.
> Eventually, we might reconsider this feature in its
> entirety, but for now we can't remove it as it would break
> a bunch of users. Instead, try to limit it to non-C++ symbols,
> where getting the invariants right is hopefully easier.
>
> Now you can't do in lldb anymore
> (lldb) call _Zsomethingsomething(1,2,3)
>
> but that doesn't seem to be such a big loss.
>
>
> I’m somewhat surprised by this. My understanding of the crash you were
> investigating is that Clang crashed because we injected a Decl looking like
> this: “void operator==(…)” after finding the operator== symbol somewhere. I
> think injecting bogus C++ Decls makes no sense and it cannot really work
> anyway.
>
> On the other hand, I see no harm in injecting “_Zsomethingsomething(…)” as a
> C Decl. This can be useful, and people should be able to call raw symbols in
> their binaries. Is there no way to keep the later while preventing the
> creation of broken C++ decls?
>
>
> Thank you all for your feedback. I'll reply with a single mail to everybody.
>
> C decls can be inserted. In fact, this works, even after my changes:
>
> (lldb) call printf("patatino")
> (int) $0 = 8
>
>
> That is because we define a function prototype for printf. So please try
> another function. You always have to cast the result of a function call to a
> symbol. In this case, since you didn't get a warning, you can know that you
> didn't use a symbol...
>
>
> I always thought identifiers beginning with underscore where illegal in C.
> Here's what the standard says:
>
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf
>
> Section 7.1.3
> "All identifiers that begin with an underscore and either an uppercase
> letter or another underscore are always reserved for any use."
>
>
> And yet there are many symbols that violate this. Just because someone says
> it is so, doesn't mean it is and also if there is no enforcement, then the
> rule doesn't mean much.
>
>
> They're not quite illegal, but they're reserved, so I'm unsure how
> frequent having symbols starting with `_Z` is popular.
>
>
> It is a legal C identifier, so we need to be able to call it.
>
>
> Maybe lldb has a better way of detecting whether this is a C or a C++
> program?
>
>
> Yes with debug info, no if we only have symbol table.
>
>
> There are several constraints:
>
> 1) The object from which we're loading symbols has no debug info, so
> we can't look at the CU and just say whether it's C or C++ or
> Objective-C.
> 2) The expression parser always evaluates expressions in a C++ context
> (to the best of my understanding)
> 3) You can always mix-and-match C/C++ object files as they're just
> Mach-O or ELF objects at that point (not recommended, but I've seen
> people doing it).
>
> Do you have any thoughts on how this should be achieved?
>
>
> What are you trying to achieve?
>

We had the report of a crash where lldb was setting a conditional
breakpoint on an invalid condition (CGRect == pointer, which is,
ill-formed).
The symbol-based resolution of lldb was picking a random operator==,
inserting a generic function decl operator== in the context because it
happened to find a matching symbol somewhere in the system.

clang expects operator== to have at least one argument, so you end up
hitting this assertion in Sema.

(lldb) Assertion failed: (i < getNumParams() && "Illegal param #"),
function getParamDecl, file
/Users/davide/work/llvm-monorepo/llvm-project-20170507/llvm/tools/clang/include/clang/AST/Decl.h,
line 2232.

So, to answer your question, Greg, I just want lldb to not inject
arbitrary C++ func decl. What do you think is the best way of
achieving this?

Best,

--
Davide