[PATCH 1/3] [gdb/tetsuite] Add missing include in gdb.base/ctf-ptype.c

[PATCH 1/3] [gdb/tetsuite] Add missing include in gdb.base/ctf-ptype.c

[Tom de Vries]

On fedora rawhide, when running test-case gdb.base/ctf-ptype.exp, I get:
...
gdb compile failed, ctf-ptype.c: In function 'main':
ctf-ptype.c:242:29: error: implicit declaration of function 'malloc' \
  [-Wimplicit-function-declaration]
  242 |   v_char_pointer = (char *) malloc (1);
      |                             ^~~~~~
ctf-ptype.c:1:1: note: include '<stdlib.h>' or provide a declaration of 'malloc'
  +++ |+#include <stdlib.h>
    1 | /* This test program is part of GDB, the GNU debugger.
...

Fix this by adding the missing include.

Tested on aarch64-linux.
---
 gdb/testsuite/gdb.base/ctf-ptype.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.base/ctf-ptype.c b/gdb/testsuite/gdb.base/ctf-ptype.c
index 4d2df33c53c..ca347893349 100644
--- a/gdb/testsuite/gdb.base/ctf-ptype.c
+++ b/gdb/testsuite/gdb.base/ctf-ptype.c
@@ -24,6 +24,8 @@
  *	First the basic C types.
  */
 
+#include <stdlib.h>
+
 #if !defined (__STDC__) && !defined (_AIX)
 #define signed  /**/
 #endif
@@ -234,9 +236,6 @@ func_type v_func_type;
 
 int main ()
 {
-  /* Ensure that malloc is a pointer type; avoid use of "void" and any include files. */
-/*  extern char *malloc();*/
-
   /* Some of the tests in ptype.exp require invoking malloc, so make
      sure it is linked in to this program.  */
   v_char_pointer = (char *) malloc (1);

base-commit: 18d2988e5da8919514c76b83e2c0b56e439018bd
-- 
2.35.3

[PATCH 2/3] [gdb/testsuite] Fix missing return type in gdb.linespec/break-asm-file.c

[Tom de Vries]

On fedora rawhide, when running test-case gdb.linespec/break-asm-file.exp, I
get:
...
gdb compile failed, break-asm-file.c:21:8: error: \
  return type defaults to 'int' [-Wimplicit-int]
   21 | static func()
      |        ^~~~
...

Fix this by adding the missing return type.

Tested on aarch64-linux.
---
 gdb/testsuite/gdb.linespec/break-asm-file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.linespec/break-asm-file.c b/gdb/testsuite/gdb.linespec/break-asm-file.c
index 5ba964de1a3..7aff7cc70ed 100644
--- a/gdb/testsuite/gdb.linespec/break-asm-file.c
+++ b/gdb/testsuite/gdb.linespec/break-asm-file.c
@@ -18,7 +18,7 @@
 void func3();
 void func2();
 
-static func()
+static void func()
 {
 }
 
-- 
2.35.3

[PATCH 3/3] [gdb/testsuite] Add missing includes in gdb.trace/collection.c

[Tom de Vries]

On fedora rawhide, with test-case gdb.trace/collection.exp, I get:
...
gdb compile failed, collection.c: In function 'strings_test_func':
collection.c:227:13: error: implicit declaration of function 'malloc' \
  [-Wimplicit-function-declaration]
  227 |   longloc = malloc(500);
      |             ^~~~~~
collection.c:1:1: note: \
  include '<stdlib.h>' or provide a declaration of 'malloc'
  +++ |+#include <stdlib.h>
    1 | /* This testcase is part of GDB, the GNU debugger.

collection.c:228:3: error: implicit declaration of function 'strcpy' \
  [-Wimplicit-function-declaration]
  228 |   strcpy(longloc, ... );
      |   ^~~~~~
collection.c:1:1: note: include '<string.h>' or provide a declaration of \
  'strcpy'
  +++ |+#include <string.h>
    1 | /* This testcase is part of GDB, the GNU debugger.
collection.c:230:8: error: implicit declaration of function 'strlen' \
  [-Wimplicit-function-declaration]
  230 |   i += strlen (locstr);
      |        ^~~~~~
collection.c:230:8: note: include '<string.h>' or provide a declaration of \
  'strlen'
...

Fix this by adding the missing includes.

Tested on aarch64-linux.
---
 gdb/testsuite/gdb.trace/collection.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdb/testsuite/gdb.trace/collection.c b/gdb/testsuite/gdb.trace/collection.c
index efcc2e389da..8379b7d7eaf 100644
--- a/gdb/testsuite/gdb.trace/collection.c
+++ b/gdb/testsuite/gdb.trace/collection.c
@@ -19,6 +19,9 @@
  * Test program for trace collection
  */
 
+#include <string.h>
+#include <stdlib.h>
+
 /*
  * Typedefs
  */
-- 
2.35.3

Re: [PATCH 1/3] [gdb/tetsuite] Add missing include in gdb.base/ctf-ptype.c

[John Baldwin]

On 3/27/24 10:30 AM, Tom de Vries wrote:
> On fedora rawhide, when running test-case gdb.base/ctf-ptype.exp, I get:
> ...
> gdb compile failed, ctf-ptype.c: In function 'main':
> ctf-ptype.c:242:29: error: implicit declaration of function 'malloc' \
>    [-Wimplicit-function-declaration]
>    242 |   v_char_pointer = (char *) malloc (1);
>        |                             ^~~~~~
> ctf-ptype.c:1:1: note: include '<stdlib.h>' or provide a declaration of 'malloc'
>    +++ |+#include <stdlib.h>
>      1 | /* This test program is part of GDB, the GNU debugger.
> ...
> 
> Fix this by adding the missing include.
> 
> Tested on aarch64-linux.

Patches 2 and 3 look obvious to me (so Approved-By on this is fine by me)

> ---
>   gdb/testsuite/gdb.base/ctf-ptype.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/ctf-ptype.c b/gdb/testsuite/gdb.base/ctf-ptype.c
> index 4d2df33c53c..ca347893349 100644
> --- a/gdb/testsuite/gdb.base/ctf-ptype.c
> +++ b/gdb/testsuite/gdb.base/ctf-ptype.c
> @@ -24,6 +24,8 @@
>    *	First the basic C types.
>    */
>   
> +#include <stdlib.h>
> +
>   #if !defined (__STDC__) && !defined (_AIX)
>   #define signed  /**/
>   #endif
> @@ -234,9 +236,6 @@ func_type v_func_type;
>   
>   int main ()
>   {
> -  /* Ensure that malloc is a pointer type; avoid use of "void" and any include files. */
> -/*  extern char *malloc();*/
> -

This certainly seems curious.  Nothing in the commit log for when this was added
gives any clue to why this comment is here, nor why the prototype is commented
out.  Other comments later in this function mention limitations of the linker on
AIX, which makes me wonder how it was tested originally.  Did you confirm that
the test program compiled contained CTF but not DWARF when the test passed?

-- 
John Baldwin

Re: [PATCH 1/3] [gdb/tetsuite] Add missing include in gdb.base/ctf-ptype.c

[Tom de Vries]

On 3/28/24 18:21, John Baldwin wrote:
> On 3/27/24 10:30 AM, Tom de Vries wrote:
>> On fedora rawhide, when running test-case gdb.base/ctf-ptype.exp, I get:
>> ...
>> gdb compile failed, ctf-ptype.c: In function 'main':
>> ctf-ptype.c:242:29: error: implicit declaration of function 'malloc' \
>>    [-Wimplicit-function-declaration]
>>    242 |   v_char_pointer = (char *) malloc (1);
>>        |                             ^~~~~~
>> ctf-ptype.c:1:1: note: include '<stdlib.h>' or provide a declaration 
>> of 'malloc'
>>    +++ |+#include <stdlib.h>
>>      1 | /* This test program is part of GDB, the GNU debugger.
>> ...
>>
>> Fix this by adding the missing include.
>>
>> Tested on aarch64-linux.
> 
> Patches 2 and 3 look obvious to me (so Approved-By on this is fine by me)
> 

Hi John,

thanks for the reviews.

I've committed these two.

>> ---
>>   gdb/testsuite/gdb.base/ctf-ptype.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/ctf-ptype.c 
>> b/gdb/testsuite/gdb.base/ctf-ptype.c
>> index 4d2df33c53c..ca347893349 100644
>> --- a/gdb/testsuite/gdb.base/ctf-ptype.c
>> +++ b/gdb/testsuite/gdb.base/ctf-ptype.c
>> @@ -24,6 +24,8 @@
>>    *    First the basic C types.
>>    */
>> +#include <stdlib.h>
>> +
>>   #if !defined (__STDC__) && !defined (_AIX)
>>   #define signed  /**/
>>   #endif
>> @@ -234,9 +236,6 @@ func_type v_func_type;
>>   int main ()
>>   {
>> -  /* Ensure that malloc is a pointer type; avoid use of "void" and 
>> any include files. */
>> -/*  extern char *malloc();*/
>> -
> 
> This certainly seems curious.  Nothing in the commit log for when this 
> was added
> gives any clue to why this comment is here, nor why the prototype is 
> commented
> out.  Other comments later in this function mention limitations of the 
> linker on
> AIX, which makes me wonder how it was tested originally.  Did you 
> confirm that
> the test program compiled contained CTF but not DWARF when the test passed?
> 

I did not, but I've done so now, I tested this on fedora asahi 39, 
aarch64-linux, and there's no dwarf in the exec, only ctf.

My guess of what happened during development of the test-case is:
- copy ptype.c to ctf-ptype.c
- run into some problem with including stdlib.h
- remove the include
- anticipate that compilation will fail due to malloc defaulting to
   int return type, and add comment and declaration
- either:
   - accidentally comment out declaration, or
   - intentionally comment out declaration, see that compilation actually
     works because a compiler-builtin declaration is used, rely on it and
     forget to update the comment to point that out

Avoiding void also doesn't make sense, given that the same commit that 
introduces the comment also introduces void.

Anyway, since it was commented out we've been using the gcc builtin 
prototype which has a void * return type.

So, I think reverting to using the include is the right thing to do.

It's possible the include was removed in early stages of support in the 
compiler or gdb where pulling in a header file pulled in unsupported stuff.

If there is indeed a problem, it'll pop up and we'll be able to fix it 
and add a proper comment explaining why we should avoid void or include 
files.

Thanks,
- Tom	

Re: [PATCH 1/3] [gdb/tetsuite] Add missing include in gdb.base/ctf-ptype.c

[Tom Tromey]

>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:

>> int main ()
>> {
>> -  /* Ensure that malloc is a pointer type; avoid use of "void" and any include files. */
>> -/*  extern char *malloc();*/
>> -

John> This certainly seems curious.  Nothing in the commit log for when this was added
John> gives any clue to why this comment is here, nor why the prototype is commented
John> out.  Other comments later in this function mention limitations of the linker on
John> AIX, which makes me wonder how it was tested originally.  Did you confirm that
John> the test program compiled contained CTF but not DWARF when the test passed?

This file was apparently just copy-pasted from ptype.c, which has this
same commented-out line.  It probably dates to the stabs days.  I
wouldn't worry too much about it... it could use a scrubbing and perhaps
should be moved to gdb.ctf anyway.

Tom

Re: [PATCH 1/3] [gdb/tetsuite] Add missing include in gdb.base/ctf-ptype.c

[Tom de Vries]

On 4/1/24 17:49, Tom Tromey wrote:
>>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:
> 
>>> int main ()
>>> {
>>> -  /* Ensure that malloc is a pointer type; avoid use of "void" and any include files. */
>>> -/*  extern char *malloc();*/
>>> -
> 
> John> This certainly seems curious.  Nothing in the commit log for when this was added
> John> gives any clue to why this comment is here, nor why the prototype is commented
> John> out.  Other comments later in this function mention limitations of the linker on
> John> AIX, which makes me wonder how it was tested originally.  Did you confirm that
> John> the test program compiled contained CTF but not DWARF when the test passed?
> 
> This file was apparently just copy-pasted from ptype.c, which has this
> same commented-out line.  It probably dates to the stabs days.  I
> wouldn't worry too much about it... it could use a scrubbing and perhaps
> should be moved to gdb.ctf anyway.

OK, in that case ... pushed.

Thanks,
- Tom