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

[Davide Italiano via lldb-commits]

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

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."

They're not quite illegal, but they're reserved, so I'm unsure how
frequent having symbols starting with `_Z` is popular.

Maybe lldb has a better way of detecting whether this is a C or a C++ program?

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?

Best,

--
Davide