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

[Davide Italiano via lldb-commits]

Fixed in a nicer/cleaner way (that doesn't regress the current
behavior), thank you everybody for your excellent feedback!

davide at Davidinos-Mac-Pro ~/w/l/llvm-project-20170507> git llvm push
Pushing 1 commit:
  8875fcce772 [ExpressionParser] Re-implement r327356 in a less disruptive way.
Sending        lldb/trunk/include/lldb/Symbol/ClangASTContext.h
Deleting       lldb/trunk/lit/Expr/Inputs/basic.cpp
Deleting       lldb/trunk/lit/Expr/TestCallCppSym.test
Adding         lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/operator-overload
Adding         lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/operator-overload/Makefile
Adding         lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/operator-overload/TestOperatorOverload.py
Adding         lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/operator-overload/a.cpp
Adding         lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/operator-overload/b.cpp
Sending        lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
Sending        lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
Sending        lldb/trunk/source/Symbol/ClangASTContext.cpp
Transmitting file data ........done
Committing transaction...
Committed revision 328025.
Committed 8875fcce772 to svn.

Thanks,

--
Davide

On Thu, Mar 15, 2018 at 8:36 AM, Davide Italiano <dccitaliano at gmail.com> wrote:
> On Wed, Mar 14, 2018 at 1:52 AM, Pavel Labath <labath at google.com> wrote:
>> I'm not familiar with all of the magic we do when we synthesize clang Decls,
>> but I feel I should point out that we can't get out of business of
>> sanity-checking the declarations we inject into clang. The reason for that
>> is, even if we had debug info for operator==, the debug info itself could
>> describe it's prototype as operator==(...) (due to a compiler bug, corrupt
>> file, or whatever). So we still need to make sure that the declarations we
>> synthesize from debug info don't violate clang's invariants (and that's what
>> we try to do at present, cf.
>> ClangASTContext::CheckOverloadedOperatorParameterCount).
>>
>> So maybe the solution here is not to refuse injecting any declarations
>> without debug info, but instead to make sure that whatever declarations we
>> inject that way satisfy the same validity criteria as the ones we synthesize
>> from the debug info?
>>
>
> I'll think about this more. On a more practical note, I was a able to
> reproduce this with a fairly self contained C++ program :)
>
> dcci at Davides-MacBook-Pro ~/w/l/b/bin> cat patatino.cpp
> class Patatino {
> public:
>   double _blah;
>   Patatino(int blah) : _blah(blah) {}
> };
>
> bool operator==(const Patatino& a, const Patatino& b) {
>   return a._blah < b._blah;
> }
>
>
> dcci at Davides-MacBook-Pro ~/w/l/b/bin> cat patatuccio.cpp
> class Baciotto {
> public:
>   int _meh;
>   Baciotto(int meh) : _meh(meh) {}
> };
>
> int main(void) {
>   Baciotto x(12);
>   return 0;
> }
>
>
> $ ./clang++ patatuccio.cpp -o patatuccio.o -c -g
> $ ./clang++ patatino.cpp -o patatino.o -c
> $ ./clang++ patatino.o patatuccio.o -o patatuccio
>
> $ nm ./patatuccio
> 0000000100000f70 t __ZN8BaciottoC1Ei
> 0000000100000fa0 t __ZN8BaciottoC2Ei.
> 0000000100000f10 T __ZeqRK8PatatinoS1_.  <--- this is the wrong symbol picked up
> 0000000100000000 T __mh_execute_header
> 0000000100000f40 T _main
>                  U dyld_stub_binder
>
> $ echo '__ZeqRK8PatatinoS1_' | c++filt
> operator==(Patatino const&, Patatino const&)
>
> And in lldb:
>
> (lldb) n
> Process 35027 stopped
> * thread #1, queue = 'com.apple.main-thread', stop reason = step over
>     frame #0: 0x0000000100000f5f patatuccio`main at patatuccio.cpp:9
>    6
>    7   int main(void) {
>    8    Baciotto x(12);
> -> 9    return 0;
>    10   }
> (lldb) expr x == nil
> Assertion failed: (i < getNumParams() && "Illegal param #"), function
> getParamDecl, file
> /Users/dcci/work/llvm/llvm/tools/clang/include/clang/AST/Decl.h, line
> 2232.
> fish: './lldb' terminated by signal SIGABRT (Abort)
>
>
> I'll try debugging this more.
>
> Thanks!
>
> --
> Davide