This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Change in binutils-gdb[master]: [gdb/tdep] Fix inferior call arg passing for amd64
- From: "Tom de Vries (Code Review)" <gerrit at gnutoolchain-gerrit dot osci dot io>
- To: gdb-patches at sourceware dot org
- Cc: Simon Marchi <simon dot marchi at polymtl dot ca>
- Date: Tue, 15 Oct 2019 05:11:17 -0400
- Subject: Change in binutils-gdb[master]: [gdb/tdep] Fix inferior call arg passing for amd64
- Auto-submitted: auto-generated
- References: <gerrit.1571043259000.Id55c74755f0a431ce31223acc86865718ae0c123@gnutoolchain-gerrit.osci.io>
- Reply-to: tdevries at suse dot de, simon dot marchi at polymtl dot ca, gdb-patches at sourceware dot org
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