[Bug c++/17004] New: print base::operator char * passes in cpexprs.exp, but only accidentally

[Bug c++/17004] New: print base::operator char * passes in cpexprs.exp, but only accidentally

[dje at google dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17004

            Bug ID: 17004
           Summary: print base::operator char * passes in cpexprs.exp, but
                    only accidentally
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: c++
          Assignee: unassigned at sourceware dot org
          Reporter: dje at google dot com

I happened to create a gdb that failed this testcase in cpexprs.exp.

print base::operator char*

Looking at a pristine copy of HEAD to see what's going on I found that this
test passes, but only accidentally.

Parsing of the expression creates "operatorchar *" as the member fn to look up
in "base".  Note no space after operator, which is fine for operator+, etc.
It's a little confusing for "operator char *", though not a bug per se.

c-exp.y:

        |       OPERATOR conversion_type_id
                        { char *name;
                          long length;
                          struct ui_file *buf = mem_fileopen ();

                          c_print_type ($2, NULL, buf, -1, 0,
                                        &type_print_raw_options);
                          name = ui_file_xstrdup (buf, &length);
                          ui_file_delete (buf);
                          $$ = operator_stoken (name);
                          free (name);
                        }

strcmp is used to do member fn comparisons here in
valops.c:value_struct_elt_for_reference:

    for (i = TYPE_NFN_FIELDS (t) - 1; i >= 0; --i)
      {
        const char *t_field_name = TYPE_FN_FIELDLIST_NAME (t, i);
        char dem_opname[64];

        if (strncmp (t_field_name, "__", 2) == 0
            || strncmp (t_field_name, "op", 2) == 0
            || strncmp (t_field_name, "type", 4) == 0)
          {
            if (cplus_demangle_opname (t_field_name,
                                       dem_opname, DMGL_ANSI))
              t_field_name = dem_opname;
            else if (cplus_demangle_opname (t_field_name,
                                            dem_opname, 0))
              t_field_name = dem_opname;
          }
        if (t_field_name && strcmp (t_field_name, name) == 0)

That fails, t_field_name is "operator char *".
Later as a last resort we try this:

    /* As a last chance, pretend that CURTYPE is a namespace, and look          
       it up that way; this (frequently) works for types nested inside          
       classes.  */

    return value_maybe_namespace_elt (curtype, name,
                                      want_address, noside);

And that works (for reasons I didn't look into).

One could argue the bug is t_field_name should be "operatorchar *".
Maybe cplus_demangle_opname is responsible for doing that.
I'm not sure.
Still, IWBN if it were spelled "operator char *".

Or maybe we should be using strcmp_iw here instead of strcmp.
[Not my preferred solution - I'd rather be strict, requiring strcmp, but my
opinion may change as I dig deeper - though this bug has a low priority for me
right now.]

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug c++/17004] print base::operator char * passes in cpexprs.exp, but only accidentally

[dje at google dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17004

--- Comment #1 from dje at google dot com ---
SamB pointed out on IRC that converting "base::operator char *" to
"base::operatorchar *" can be ambiguous if the class also really has a member
base::operatorchar.

Plus it's not clear to me how much canonicalization is strictly adhered to and
how much gdb just gets away with bugs because of how it works.
E.g. dwarf2read.c:mapped_index_string_hash doesn't ignore spaces.
So if the canonicalization is wrong, the symbol won't be found in the index.
However, it probably won't ever matter, at least in the case of member
functions, because gdb will always lookup the class first, and thus expand the
symbol table containing the class before it ever looks up a member function in
that class.  And because gdb looks in already expanded symbol tables before
trying the index, any improper canonicalization that prevents symbols being
found in the index won't ever matter.

Another reason why such bugs can get missed is because of the bazillion
different ways gdb tries to lookup a symbol if one way fails.  Here again a
lookup in the index can errantly fail, but because the symtab got expanded
during the first attempt then when gdb tries to lookup the symbol in a
different way (see, e.g., cp-namespace.c) subsequent lookups will get to use
expanded symtabs.

This matters because I'm looking into having gdb *only* use the index for
lookups. Still don't know if this path will be fruitful, but iterating over
already expanded symtabs when .gdb_index already knows exactly which symtabs to
look at feels promising.
[It also matters of course because it just makes gdb hard to hack on.]

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug c++/17004] print base::operator char * passes in cpexprs.exp, but only accidentally

[dje at google dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17004

--- Comment #2 from dje at google dot com ---
Created attachment 7626
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7626&action=edit
example to trigger operator not found bug

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug c++/17004] print base::operator char * passes in cpexprs.exp, but only accidentally

[dje at google dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17004

--- Comment #3 from dje at google dot com ---
The attached example shows gdb not able to find operator char *, due to it
being looked up as operatorchar *.  The key is to not cause the symtab with the
operator definition to be prematurely expanded, forcing gdb to try to look up
the operator in .gdb_index.
The attached example puts main in one .o, the class definition in the next, and
the operator definition in the last.

(gdb) p base::operator char*
There is no field named operatorchar *

-- 
You are receiving this mail because:
You are on the CC list for the bug.