libsanitizer: Fix setbuffer() interceptor (accept size not mode)

Message ID 20211222181912.29879-1-a3at.mail@gmail.com
State New
Headers
Series libsanitizer: Fix setbuffer() interceptor (accept size not mode) |

Commit Message

Azat Khuzhin Dec. 22, 2021, 6:19 p.m. UTC
  Fixes: b667dd7017a ("Libsanitizer merge from trunk r368656.")
Refs: https://reviews.llvm.org/D116176
---
 .../sanitizer_common/sanitizer_common_interceptors.inc     | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
  

Comments

Bernhard Reutner-Fischer Dec. 22, 2021, 8:41 p.m. UTC | #1
On 22 December 2021 19:19:12 CET, Azat Khuzhin via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>Fixes: b667dd7017a ("Libsanitizer merge from trunk r368656.")
>Refs: https://reviews.llvm.org/D116176
>---
> .../sanitizer_common/sanitizer_common_interceptors.inc     | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc b/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc
>index abb38ccfa15..60b0545a943 100644
>--- a/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc
>+++ b/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc
>@@ -7858,12 +7858,13 @@ INTERCEPTOR(void, setbuf, __sanitizer_FILE *stream, char *buf) {
>       unpoison_file(stream);
> }
> 
>-INTERCEPTOR(void, setbuffer, __sanitizer_FILE *stream, char *buf, int mode) {
>+INTERCEPTOR(void, setbuffer, __sanitizer_FILE *stream, char *buf,
>+  SIZE_T size) {
>   void *ctx;
>-  COMMON_INTERCEPTOR_ENTER(ctx, setbuffer, stream, buf, mode);
>+  COMMON_INTERCEPTOR_ENTER(ctx, setbuffer, stream, buf, size);
>   REAL(setbuffer)(stream, buf, mode);

Where does mode come from after this patch?
thanks,

>   if (buf) {
>-    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, __sanitizer_bufsiz);
>+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, size);
>   }
>   if (stream)
>     unpoison_file(stream);
  
Azat Khuzhin Dec. 22, 2021, 8:45 p.m. UTC | #2
On Wed, Dec 22, 2021 at 09:41:06PM +0100, Bernhard Reutner-Fischer wrote:
> On 22 December 2021 19:19:12 CET, Azat Khuzhin via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> >Fixes: b667dd7017a ("Libsanitizer merge from trunk r368656.")
> >Refs: https://reviews.llvm.org/D116176
> >---
> > .../sanitizer_common/sanitizer_common_interceptors.inc     | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> >diff --git a/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc b/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc
> >index abb38ccfa15..60b0545a943 100644
> >--- a/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc
> >+++ b/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc
> >@@ -7858,12 +7858,13 @@ INTERCEPTOR(void, setbuf, __sanitizer_FILE *stream, char *buf) {
> >       unpoison_file(stream);
> > }
> > 
> >-INTERCEPTOR(void, setbuffer, __sanitizer_FILE *stream, char *buf, int mode) {
> >+INTERCEPTOR(void, setbuffer, __sanitizer_FILE *stream, char *buf,
> >+  SIZE_T size) {
> >   void *ctx;
> >-  COMMON_INTERCEPTOR_ENTER(ctx, setbuffer, stream, buf, mode);
> >+  COMMON_INTERCEPTOR_ENTER(ctx, setbuffer, stream, buf, size);
> >   REAL(setbuffer)(stream, buf, mode);
> 
> Where does mode come from after this patch?

setbuffer() does not accept mode, it simply do not change it.
Only setvbuf() can change the mode.

> thanks,
> 
> >   if (buf) {
> >-    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, __sanitizer_bufsiz);
> >+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, size);
> >   }
> >   if (stream)
> >     unpoison_file(stream);
>
  
Azat Khuzhin Dec. 22, 2021, 8:50 p.m. UTC | #3
On Wed, Dec 22, 2021 at 09:41:06PM +0100, Bernhard Reutner-Fischer wrote:
> On 22 December 2021 19:19:12 CET, Azat Khuzhin via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> >-  COMMON_INTERCEPTOR_ENTER(ctx, setbuffer, stream, buf, mode);
> >+  COMMON_INTERCEPTOR_ENTER(ctx, setbuffer, stream, buf, size);
> >   REAL(setbuffer)(stream, buf, mode);
> 
> Where does mode come from after this patch?

Sorry, missed that.
Fixed and send v2 patch.

Thanks!
  
Bernhard Reutner-Fischer Dec. 22, 2021, 9:02 p.m. UTC | #4
On Wed, 22 Dec 2021 23:50:39 +0300
Azat Khuzhin <a3at.mail@gmail.com> wrote:

> Thanks!

you're welcome.

You should state how you tested the patch. Please refer to
https://gcc.gnu.org/contribute.html#testing

thanks,
  
Azat Khuzhin Dec. 22, 2021, 9:09 p.m. UTC | #5
On Wed, Dec 22, 2021 at 10:02:02PM +0100, Bernhard Reutner-Fischer wrote:
> You should state how you tested the patch. Please refer to
> https://gcc.gnu.org/contribute.html#testing

I though about this, but when gcc syncs changes with upstream [1], it
does not syncs tests, even though they were there [2].

  [1]: https://github.com/gcc-mirror/gcc/commit/b667dd7017a8
  [2]: https://github.com/llvm/llvm-project/commit/0c81a62d9d76

That's why I though that test can be ommitted in this case (I've added a
test for llvm in [3]).

  [3]: https://reviews.llvm.org/D116176

So what is the right way here?
- migrate all tests
- write test only for setbuffer()
- do not add any tests, since they are covered in llvm repo
  
Martin Liška Dec. 23, 2021, 12:07 p.m. UTC | #6
On 12/22/21 22:09, Azat Khuzhin via Gcc-patches wrote:
> So what is the right way here?
> - migrate all tests
> - write test only for setbuffer()
> - do not add any tests, since they are covered in llvm repo

Hello.

Yes, we don't automatically sync sanitizer tests when we merge from master.
Historically, we have taken some upstream tests, but not all. Problem is that
one needs to migrate (port) LIT markup to DejaGNU format so that it can be supported
in the GCC test-suite.

Cheers,
Martin
  

Patch

diff --git a/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc b/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc
index abb38ccfa15..60b0545a943 100644
--- a/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc
+++ b/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc
@@ -7858,12 +7858,13 @@  INTERCEPTOR(void, setbuf, __sanitizer_FILE *stream, char *buf) {
       unpoison_file(stream);
 }
 
-INTERCEPTOR(void, setbuffer, __sanitizer_FILE *stream, char *buf, int mode) {
+INTERCEPTOR(void, setbuffer, __sanitizer_FILE *stream, char *buf,
+  SIZE_T size) {
   void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, setbuffer, stream, buf, mode);
+  COMMON_INTERCEPTOR_ENTER(ctx, setbuffer, stream, buf, size);
   REAL(setbuffer)(stream, buf, mode);
   if (buf) {
-    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, __sanitizer_bufsiz);
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, size);
   }
   if (stream)
     unpoison_file(stream);