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

[Greg Clayton via lldb-commits]

Thanks for fixing this is the right way and taking the time!

Greg


> On Mar 20, 2018, at 12:49 PM, Davide Italiano via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> 
> 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
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits