Change in binutils-gdb[master]: [gdb/tdep] Fix inferior call arg passing for amd64

["Tom de Vries (Code Review)" <gerrit at gnutoolchain-gerrit dot osci dot io>]

Tom de Vries has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/31
......................................................................


Patch Set 1:

(1 comment)

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/31/1/gdb/amd64-tdep.c 
File gdb/amd64-tdep.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/31/1/gdb/amd64-tdep.c@645 
PS1, Line 645:     theclass[1] = amd64_merge_classes (theclass[1], subclass[1]);
> If we reach here, isn't subclass[1] always NO_CLASS?  So this if would be unnecessary?
Doing:
...
   if (pos == 0)
-    theclass[1] = amd64_merge_classes (theclass[1], subclass[1]);
+    {
+      gdb_assert (subclass[1] == AMD64_NO_CLASS);
+      theclass[1] = amd64_merge_classes (theclass[1], subclass[1]);
+    }
...
gives 180 failure in gdb.base/infcall-nested-structs.exp.

The assert is triggered for the cases where amd64_classify sets theclass[1], in this test-case:
- long double
- double _Complex.

Structurewise, we have (leaving out the massive comment that makes it hard to read):
...
  theclass[pos] = amd64_merge_classes (theclass[pos], subclass[0]);
  if (bitsize <= 64 && pos == 0 && endpos == 1)
    theclass[1] = amd64_merge_classes (theclass[1], subclass[0]);
  if (pos == 0)
    theclass[1] = amd64_merge_classes (theclass[1], subclass[1]);
...
so 'reach here' means just pos == 0.

This would actually be more precise:
...
-  if (pos == 0)
+  if (pos == 0 && endpos == 1)
     theclass[1] = amd64_merge_classes (theclass[1], subclass[1]);
...
and therefore we could restructure to:
...
  theclass[pos] = amd64_merge_classes (theclass[pos], subclass[0]);
  if (pos == 0 && endpos == 1)
    {
      theclass[1] = amd64_merge_classes (theclass[1], subclass[1]);
      if (bitsize <= 64)
        theclass[1] = amd64_merge_classes (theclass[1], subclass[0]);
    }
...
But I don't think we want to do refactoring like this in this commit.



-- 
To view, visit https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/31
To unsubscribe, or for help writing mail filters, visit https://gnutoolchain-gerrit.osci.io/r/settings