Re: [libgomp, nvptx] Fix libgomp.c/target-5.c compilation

2018-12-13 Thread Jakub Jelinek
On Thu, Dec 13, 2018 at 01:09:25PM +0100, Tom de Vries wrote:
> 2018-12-13  Tom de Vries  
> 
>   * affinity-fmt.c (gomp_print_string): New function, factored out of ...
>   (omp_display_affinity, gomp_display_affinity_thread): ... here, and ...
>   * fortran.c (omp_display_affinity_): ... here.
>   * libgomp.h (gomp_print_string): Declare.
>   * config/nvptx/affinity-fmt.c: New file.  Include affinity-fmt.c,
>   undefining HAVE_GETPID and HAVE_GETHOSTNAME, and mapping fwrite to
>   write.

> +/* The nvptx newlib implementation does not support fwrite, but it does 
> support
> +   write.  Map fwrite to write for size == 1.  */
> +#undef fwrite
> +#define fwrite(ptr, size, nmemb, stream) \
> +  ((size) == 1 ? write (1, (ptr), (nmemb)) : 0)

Why not
  write (1, (ptr), (nmemb) * (size))
I know it doesn't handle the overflow case properly, but the callers are
using 1 and it actually isn't a security threat if it prints fewer chars
than it should.  We don't have anything where we'd rely on fwrite failing
when stuff overflows...

Ok with that change (plus adjust the comment).

Jakub


[libgomp, nvptx] Fix libgomp.c/target-5.c compilation

2018-12-13 Thread Tom de Vries
[ was: Re: [libgomp, nvptx] Disable
OMP_{DISPLAY_AFFINITY,AFFINITY_FORMAT} support ]

On 12-12-18 14:15, Jakub Jelinek wrote:
> On Wed, Dec 12, 2018 at 02:02:20PM +0100, Tom de Vries wrote:
>> This RFC patch implements that approach for getpid and gethostname (I
>> wonder though whether it's not possible to turn the corresponding
>> configure tests into link tests, which could also fix this for nvptx).
>>
>> Another problem we're running into is missing istty, pulled in by
>> fwrite. The nvptx newlib port does support write, but I'm unsure in what
>> form I should handle this.  Add configure tests for fwrite and write? Or
>> special case the files in the config/nvptx dir? Any advice here?
>>
>> Thanks,
>> - Tom
> 
>> [RFC][libgomp, nvptx] Fix target-5.c compilation
>>
>> Libgomp test-case libgomp.c/target-5.c is failing to compile with nvptx
>> accelerator due to missing:
>> - getpid
>> - gethostname
>> - isatty (pulled in by fwrite)
>> in the nvptx newlib.
>>
>> This demonstrator patch fixes that by minimally guarding all related 
>> locations
>> with ifndef LIBGOMP_OFFLOADED_ONLY, which allows the test-case (and others) 
>> to
>> pass.
> 
> I guess my preference would be something similar to what
> libgomp/config/mingw32/affinity-fmt.c does now, so override the file,
> define/undefine something in there and include the common affinity-fmt.c.
> Perhaps for the fwrite stuff define introduce a function that does that
> (say
> void
> gomp_print_string (const char *str, size_t len)
> {
>   fwrite (str, 1, len, stderr);
> },
> because it is in more than one file, make it hidden in libgomp.h.
> config/nvptx/error.c then has what you could also use in
> config/nvptx/affinity-fmt.c.
> And for hostname/getpid maybe just #undef those with a comment?

Done.

OK for trunk?

Thanks,
- Tom

[libgomp, nvptx] Fix libgomp.c/target-5.c compilation

Libgomp test-case libgomp.c/target-5.c is failing to compile when building for
x86_64 with nvptx accelerator due to missing:
- getpid
- gethostname
- isatty (pulled in by fwrite)
in the nvptx newlib.

This patch fixes the build failure by:
- adding a function gomp_print_string which limits the use of fwrite to a single
  location (affinity-fmt.c), and
- creating an nvptx version of affinity-fmt.c, which:
  - implements fwrite using write
  - overrides the configure test results HAVE_GETPID and HAVE_GETHOSTNAME

Build and reg-tested on x86_64 with nvptx accelerator.

2018-12-13  Tom de Vries  

	* affinity-fmt.c (gomp_print_string): New function, factored out of ...
	(omp_display_affinity, gomp_display_affinity_thread): ... here, and ...
	* fortran.c (omp_display_affinity_): ... here.
	* libgomp.h (gomp_print_string): Declare.
	* config/nvptx/affinity-fmt.c: New file.  Include affinity-fmt.c,
	undefining HAVE_GETPID and HAVE_GETHOSTNAME, and mapping fwrite to
	write.

---
 libgomp/affinity-fmt.c  | 14 +++---
 libgomp/config/nvptx/affinity-fmt.c | 52 +
 libgomp/fortran.c   |  4 +--
 libgomp/libgomp.h   |  1 +
 4 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/libgomp/affinity-fmt.c b/libgomp/affinity-fmt.c
index 9e5c5f754eb..1447597e7f7 100644
--- a/libgomp/affinity-fmt.c
+++ b/libgomp/affinity-fmt.c
@@ -37,6 +37,12 @@
 #include 
 #endif
 
+void
+gomp_print_string (const char *str, size_t len)
+{
+  fwrite (str, 1, len, stderr);
+}
+
 void
 gomp_set_affinity_format (const char *format, size_t len)
 {
@@ -456,13 +462,13 @@ omp_display_affinity (const char *format)
   if (ret < sizeof buf)
 {
   buf[ret] = '\n';
-  fwrite (buf, 1, ret + 1, stderr);
+  gomp_print_string (buf, ret + 1);
   return;
 }
   b = gomp_malloc (ret + 1);
   ialias_call (omp_capture_affinity) (b, ret + 1, format);
   b[ret] = '\n';
-  fwrite (b, 1, ret + 1, stderr);
+  gomp_print_string (b, ret + 1);
   free (b);
 }
 
@@ -477,13 +483,13 @@ gomp_display_affinity_thread (gomp_thread_handle handle,
   if (ret < sizeof buf)
 {
   buf[ret] = '\n';
-  fwrite (buf, 1, ret + 1, stderr);
+  gomp_print_string (buf, ret + 1);
   return;
 }
   b = gomp_malloc (ret + 1);
   gomp_display_affinity (b, ret + 1, gomp_affinity_format_var,
   			 handle, ts, place);
   b[ret] = '\n';
-  fwrite (b, 1, ret + 1, stderr);
+  gomp_print_string (b, ret + 1);
   free (b);
 }
diff --git a/libgomp/config/nvptx/affinity-fmt.c b/libgomp/config/nvptx/affinity-fmt.c
new file mode 100644
index 000..ff1cbfcc4f8
--- /dev/null
+++ b/libgomp/config/nvptx/affinity-fmt.c
@@ -0,0 +1,52 @@
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This file is part of the GNU Offloading and Multi Processing Library
+   (libgomp).
+
+   Libgomp is free software; you can redistribute it and