Re: [PATCH 1/4] gdb.base/nodebug.exp: Add long double testing
Andrew Burgess <[email protected]> Wed, 22 Jul 2026 19:41:46 +0100
| Newsgroups | gmane.comp.gdb.patches |
|---|---|
| Message-ID | <[email protected]> |
Pedro Alves <[email protected]> writes: > On 2026-07-16 22:14, Andrew Burgess wrote: >> Pedro Alves <[email protected]> writes: > ... > >> Not that it really matters, but the ordering seems a bit weird, the >> existing functions are all: >> >> X >> X_noproto >> Y >> Y_noproto >> etc... >> >> but you broke this pattern. > > Whoops, that was not intentional. I think I misread the pattern earlier. I reordered it now: > > $ grep "^mult" testsuite/gdb.base/nodebug.c > multf (float v1, float v2) > multf_noproto (v1, v2) > mult (double v1, double v2) > mult_noproto (v1, v2) > mult_long_double (long double v1, long double v2) > mult_long_double_noproto (v1, v2) > > Thanks for spotting this. > >> I only mention it because I have some >> actual worthwhile points to raise below... >> >>> + >>> uint8_t >>> add8 (uint8_t v1, uint8_t v2) >>> { >>> diff --git a/gdb/testsuite/gdb.base/nodebug.exp b/gdb/testsuite/gdb.base/nodebug.exp >>> index e4138a801da..1a16e86ed2c 100644 >>> --- a/gdb/testsuite/gdb.base/nodebug.exp >>> +++ b/gdb/testsuite/gdb.base/nodebug.exp >>> @@ -78,6 +78,12 @@ proc test_call_promotion {} { >>> gdb_test "p ((double (*) ()) mult_noproto)(2.0f, 3.0f)" " = 6" >>> gdb_test "p ((double (*) ()) mult_noproto)(2.0, 3.0)" " = 6" >>> >>> + # Same, but for long double. >>> + gdb_test "p (long double) mult_long_double(2.0L, 3.0L)" " = 6" >>> + gdb_test "p ((long double (*) (long double, long double)) mult_long_double)(2.0L, 3.0L)" " = 6" >>> + gdb_test "p ((long double (*) (long double, long double)) mult_long_double)(2, 3)" " = 6" >>> + gdb_test "p ((long double (*) ()) mult_long_double_noproto)(2.0L, 3.0L)" " = 6" >> >> I noticed that the double test also includes a case which tests float to >> double promotion, but that case is skipped here. Wouldn't it be a good >> idea to include both float to long double and double to long double >> tests here too? > > That's because the double test is exercising the standard C default argument > promotion rules, for floating point, which says that for non-prototyped functions, > float is promoted to double. > > So for this float function: > > float > multf_noproto (v1, v2) > float v1, v2; > { > return v1 * v2; > } > > called like so: > > multif_noproto (1.0f, 2.0f); > > this happens: > > 1. Caller promotes 1.0f/2.0f to double, passes the double values. > 2. Callee receives doubles. > 3. Callee's prologue converts to float, because the declared param type is float. > 4. Body uses v1/v2 as float. > > ================================ > > And for the double case: > > double > mult_noproto (v1, v2) > double v1, v2; > { > return v1 * v2; > } > > called like so: > > multi_noproto (1.0f, 2.0f); > > this happens: > > 1. Caller promotes 1.0f/2.0f to double, passes the double values. > 2. Callee receives a double. > 3. Body uses v1/v2 as double. > > ================================ > > The long double case is different. The C default promotion is float->double, NOT float->long double. > > long double > mult_long_double_noproto (v1, v2) > long double v1, v2; > { > return v1 * v2; > } > > mult_long_double_noproto (1.0f, 2.0f); > > so this happens: > > 1. Caller promotes 1.0f/2.0f to double, passes the double values. > 2. Callee receives doubles. > 3. Body uses v1/v2 as long double. (with no conversion! undefined behavior.) > > On ABIs where double and long double are distinct types/sizes, the body sees bogus/corrupt values. > > =============================== > > For the prototyped case, the calls where we pass int, like: > > p ((float (*) (float, float)) multf)(2, 3) > > cover coercion from int to declared type, which is basically testing that gdb does a cast there. > testing float -> double, etc. for the prototyped cases would not add extra coverage, as it'd just > be testing that gdb knows how to cast from float -> double. > > > > I've added some comments to the test to try to make it a little clearer. I also corrected a couple > references to "coercion" to "promotion" to be more accurate. The existing comments are using the terms > interchangeably, but that's not 100% correct. > > Let me know what you think. Sorry, missed this. Thanks for the explanation above. LGTM. Approved-By: Andrew Burgess <[email protected]> Thanks, Andrew > > From 3f7db94982bb02473e89199a93dde2d27e06014a Mon Sep 17 00:00:00 2001 > From: Pedro Alves <[email protected]> > Date: Tue, 14 Jul 2026 00:34:43 +0100 > Subject: [PATCH] gdb.base/nodebug.exp: Add long double testing > > gdb.base/nodebug.exp is missing testing calling long double functions. > This commit adds such tests. > > With a GDB that doesn't know that "long double" is 64-bit on > x86_64-pc-windows-msvc, we get: > > FAIL: gdb.base/nodebug.exp: p (long double) mult_long_double(2.0L, 3.0L) > FAIL: gdb.base/nodebug.exp: p ((long double (*) (long double, long double)) mult_long_double)(2.0L, 3.0L) > FAIL: gdb.base/nodebug.exp: p ((long double (*) (long double, long double)) mult_long_double)(2, 3) > FAIL: gdb.base/nodebug.exp: p ((long double (*) ()) mult_long_double_noproto)(2.0L, 3.0L) > > Passes cleanly on: > > - x86_64-pc-linux-gnu > - x86_64-w64-mingw32 > - x86_64-pc-windows-msvc, with the "long double" fix > > Change-Id: If9ee749187e1d30fedcba17ae12634f3bd90de2f > --- > gdb/testsuite/gdb.base/nodebug.c | 13 +++++++++++++ > gdb/testsuite/gdb.base/nodebug.exp | 21 ++++++++++++++++----- > 2 files changed, 29 insertions(+), 5 deletions(-) > > diff --git a/gdb/testsuite/gdb.base/nodebug.c b/gdb/testsuite/gdb.base/nodebug.c > index c7bc93991b8..00e854844bf 100644 > --- a/gdb/testsuite/gdb.base/nodebug.c > +++ b/gdb/testsuite/gdb.base/nodebug.c > @@ -79,6 +79,19 @@ mult_noproto (v1, v2) > return v1 * v2; > } > > +long double > +mult_long_double (long double v1, long double v2) > +{ > + return v1 * v2; > +} > + > +long double > +mult_long_double_noproto (v1, v2) > + long double v1, v2; > +{ > + return v1 * v2; > +} > + > uint8_t > add8 (uint8_t v1, uint8_t v2) > { > diff --git a/gdb/testsuite/gdb.base/nodebug.exp b/gdb/testsuite/gdb.base/nodebug.exp > index e4138a801da..7a5ab273bde 100644 > --- a/gdb/testsuite/gdb.base/nodebug.exp > +++ b/gdb/testsuite/gdb.base/nodebug.exp > @@ -51,8 +51,8 @@ proc nodebug_runto {func} { > } > > # Test calling no-debug functions involving argument types that may > -# require coercion/promotion, both prototyped and unprototyped, both > -# return-type-cast style, and function-pointer-cast styles. > +# require coercion or promotion, both prototyped and unprototyped, > +# both return-type-cast style, and function-pointer-cast styles. > proc test_call_promotion {} { > if {[target_info exists gdb,cannot_call_functions]} { > return > @@ -60,14 +60,15 @@ proc test_call_promotion {} { > > # Call prototyped function with float parameters via both > # return-type cast and function-pointer cast. This checks that > - # GDB doesn't do float->double coercion. > + # GDB doesn't do float->double promotion. > gdb_test "p (float) multf(2.0f, 3.0f)" " = 6" > - gdb_test "p ((float (*) (float, float)) multf)(2, 3)" " = 6" > gdb_test "p ((float (*) (float, float)) multf)(2.0f, 3.0f)" " = 6" > + # This tests int->float coercion. > + gdb_test "p ((float (*) (float, float)) multf)(2, 3)" " = 6" > > # Call unprototyped function with float parameters via > # function-pointer cast, only. return-type cast assumes > - # protototyped. Check that GDB does float->double coercion. > + # prototyped. Check that GDB does float->double promotion. > gdb_test "p ((float (*) ()) multf_noproto)(2.0f, 3.0f)" " = 6" > gdb_test "p ((float (*) ()) multf_noproto)(2.0, 3.0)" " = 6" > > @@ -78,6 +79,16 @@ proc test_call_promotion {} { > gdb_test "p ((double (*) ()) mult_noproto)(2.0f, 3.0f)" " = 6" > gdb_test "p ((double (*) ()) mult_noproto)(2.0, 3.0)" " = 6" > > + # Same, but for long double. > + gdb_test "p (long double) mult_long_double(2.0L, 3.0L)" " = 6" > + gdb_test "p ((long double (*) (long double, long double)) mult_long_double)(2.0L, 3.0L)" " = 6" > + gdb_test "p ((long double (*) (long double, long double)) mult_long_double)(2, 3)" " = 6" > + # For unprototyped calls, the standard C default argument > + # promotion (for floating point) applies only to float and > + # promotes to double, so test only with the expected type, using > + # L-suffixed literals to pass long doubles. > + gdb_test "p ((long double (*) ()) mult_long_double_noproto)(2.0L, 3.0L)" " = 6" > + > # Check that GDB promotes char->int correctly. > gdb_test "p /d (uint8) add8((uint8) 2, (uint8) 3)" " = 5" > gdb_test "p /d ((uint8 (*) (uint8, uint8)) add8)((uint8) 2, (uint8) 3)" " = 5" > > base-commit: 490469846dcef89fe53668bdbba73591c64bed61 > -- > 2.54.0