Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

std::string vs. string literal comparison through "expr" command fails #20605

Open Quuxplusone opened 10 years ago

Quuxplusone commented 10 years ago
Bugzilla Link PR20606
Status NEW
Importance P normal
Reported by Randy Smith (rdsmith@chromium.org)
Reported on 2014-08-09 16:16:27 -0700
Last modified on 2017-08-07 16:55:09 -0700
Version unspecified
Hardware HP Linux
CC jingham@apple.com, penryu@gmail.com, scallanan@apple.com
Fixed by commit(s)
Attachments test2.cc (147 bytes, application/octet-stream)
Blocks
Blocked by
See also
Created attachment 12872
Source file for reproduction

If you try and execute C++ code comparing a std::string with a string literal
using "expr", you get a warning and and error:

(lldb) expr abc == "abc"
error: warning: result of comparison against a string literal is unspecified
(use strncmp instead)
error: invalid operands to binary expression ('string' (aka
'std::basic_string<char, std::char_traits<char>, std::allocator<char> >') and
'const char *')
error: 1 errors parsing expression
(lldb)

That comparison works fine when compiled by clang++, despite the fact that the
first warning comes from the clang library
(tools/clang/lib/Sema/SemaExpr.cpp:7960 at r215285).  I poked around in that
function, and it seems like it's designed to warn against string literal
comparison in the large majority of cases (including this one).  So the
confusing thing here may be that clang++ doesn't warn about the comparison when
compiling (though I think that is the proper behavior, since std::string
explicitly defines a binary comparison operator of std::string vs. char*).

I did not explore the function where the second error comes from
(tools/clang/lib/Sema/SemaExpr.cpp:6798, same revision).

Reproduction: Compile the attached file with clang++ -g test2.cc (note the lack
of error or warning), and

lldb a.out
break set -f test2.cc -l 8
run
expr abc == "abc"
Quuxplusone commented 10 years ago

Attached test2.cc (147 bytes, application/octet-stream): Source file for reproduction

Quuxplusone commented 7 years ago
I've reproduced the behavior described.

(lldb) target create "a.out"
Current executable set to 'a.out' (x86_64).
(lldb) br s -l 8
Breakpoint 1: where = a.out`main + 532 at test2.cc:8, address =
0x0000000100000b94
(lldb) r
Process 80442 launched: '/Users/penryu/tmp/test2/a.out' (x86_64)
Process 80442 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
    frame #0: 0x0000000100000b94 a.out`main(argc=1, argv=0x00007fff5fbff838) at test2.cc:8
   5    main(int argc, char** argv) {
   6      std::string abc("abc");
   7
-> 8      std::cout << (abc == "def") << std::endl;
   9    }
   10
Target 0: (a.out) stopped.
(lldb) expr (abc == "abc")
error: warning: result of comparison against a string literal is unspecified
(use strncmp instead)
error: invalid operands to binary expression ('string' (aka
'std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >') and 'const char *')
(lldb) expr abc.compare("abc")
(int) $0 = 0

I wonder if this isn't a case where lldb's expression evaluator doesn't match
clang's.
Quuxplusone commented 7 years ago
From looking at the headers, it seems that the expression:

  (abc == "def")

is being expanded via templates to:

  std::basic_string<>::operator==(abc, "def")

which is then being inlined to:

  abc.compare("def")

... which is supported by fact that the binary links against the
basic_string<>::compare(char const*) method from libc++.

So it looks like there isn't any actual implementation of operator==()
available for lldb to use, though I'm not familiar with the details of how it's
expression evaluator might differ.
Quuxplusone commented 7 years ago
Yes, lldb's integration with the STL is hampered by the fact that the STL
inlines much of its implementation, leaving nothing for lldb to call.  That's
true here, and it is true for things like std::vector<anything>::size, etc...

We need to find a way to teach lldb how to reconstruct these inlined functions
from the STL headers.  By our current lights, we'll get that ability from clang
learning to compose fully function STL modules, and then teaching lldb to
ingest same.

Note, this isn't a problem with the expression parser understanding ==
operators, as you can show with simple examples like:

 > cat compare_strings.cpp
#include <string>

class Foo
{
public:
  int a;
  bool operator==(const std::string &rhs)
  {
    if (rhs.size() == a)
      return true;
    return false;
  }
};

int
main()
{
  Foo my_foo = {5};
  std::string my_str("asdfs");
  bool equals = my_foo == my_str;
  if (equals)
    return 0;
  return 1;
}
 > clang++ -g -O0 -o compare_strings compare_strings.cpp
 > lldb compare_strings
(lldb) target create "compare_strings"
Current executable set to 'compare_strings' (x86_64).
(lldb) b s -l 21
Breakpoint 1: where = compare_strings`main + 204 at compare_strings.cpp:21,
address = 0x0000000100000cec
(lldb) run
Process 25890 launched: '/private/tmp/compare_strings' (x86_64)
Process 25890 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
    frame #0: 0x0000000100000cec compare_strings`main at compare_strings.cpp:21
   18     Foo my_foo = {5};
   19     std::string my_str("asdfs");
   20     bool equals = my_foo == my_str;
-> 21     if (equals)
              ^
   22       return 0;
   23     return 1;
   24   }
Target 0: (compare_strings) stopped.
(lldb) expr my_foo == my_str
(bool) $0 = true