Re: [PATCH] BUILD: Avoid warning about ignoring write()'s return value in BUG_ON()
Willy, Am 14.03.20 um 12:15 schrieb Willy Tarreau: > Good point we could indeed. Quite frankly it's a detail since this > really is developer-only code. Nobody wants to run with DEBUG_UAF by > accident, it's slow and uses a lot of memory :-) > As I had the code open anyway to make that CLEANUP to the unique ID in proxy I also looked at this. Result: No, this cannot be trivially replaced: > include/common/memory.h: In function ‘pool_free’: > include/common/memory.h:338:4: warning: implicit declaration of function > ‘ABORT_NOW’ [-Wimplicit-function-declaration] > ABORT_NOW(); > ^ Best regards Tim Düsterhus
Re: [PATCH] BUILD: Avoid warning about ignoring write()'s return value in BUG_ON()
On Sat, Mar 14, 2020 at 11:56:13AM +0100, Tim Düsterhus wrote: > Willy, > > Am 14.03.20 um 11:14 schrieb Willy Tarreau: > > Now done. I've also cleaned up the null-derefw warning in the debugging > > code of the pools. > > > > Can't the pools simply use `ABORT_NOW()` instead of `*DISGUISE((volatile > int *)0) = 0;`? Good point we could indeed. Quite frankly it's a detail since this really is developer-only code. Nobody wants to run with DEBUG_UAF by accident, it's slow and uses a lot of memory :-) Willy
Re: [PATCH] BUILD: Avoid warning about ignoring write()'s return value in BUG_ON()
Willy, Am 14.03.20 um 11:14 schrieb Willy Tarreau: > Now done. I've also cleaned up the null-derefw warning in the debugging > code of the pools. > Can't the pools simply use `ABORT_NOW()` instead of `*DISGUISE((volatile int *)0) = 0;`? Best regards Tim Düsterhus
Re: [PATCH] BUILD: Avoid warning about ignoring write()'s return value in BUG_ON()
On Fri, Mar 13, 2020 at 07:07:54PM +0100, Willy Tarreau wrote: > On Fri, Mar 13, 2020 at 05:41:14PM +0100, Tim Düsterhus wrote: > > Willy, > > > > Am 13.03.20 um 17:34 schrieb Willy Tarreau: > > > Indeed, just found it in my queue. However we usually use it > > > differently, with the function instead of the variable. Do you > > > mind if I adapt it ? > > > > > > > I attempted to use the function, but it didn't compile. I guess because > > of a circular dependency. If it's a small obvious change feel free to > > adapt the patch. If it requires larger changes please fix it yourself > > and ignore my patch. > > OK, I'll check when I have a few minutes. I think that since we now > have this ALREADY_CHECKED() macro that's used to prevent gcc from > seeing null-derefs where they can't happen, we can use it as well > to pretend we've consumed the result from such occasional syscalls > and even remove the shut_* macro. Now done. I've also cleaned up the null-derefw warning in the debugging code of the pools. Willy
Re: [PATCH] BUILD: Avoid warning about ignoring write()'s return value in BUG_ON()
On Fri, Mar 13, 2020 at 05:41:14PM +0100, Tim Düsterhus wrote: > Willy, > > Am 13.03.20 um 17:34 schrieb Willy Tarreau: > > Indeed, just found it in my queue. However we usually use it > > differently, with the function instead of the variable. Do you > > mind if I adapt it ? > > > > I attempted to use the function, but it didn't compile. I guess because > of a circular dependency. If it's a small obvious change feel free to > adapt the patch. If it requires larger changes please fix it yourself > and ignore my patch. OK, I'll check when I have a few minutes. I think that since we now have this ALREADY_CHECKED() macro that's used to prevent gcc from seeing null-derefs where they can't happen, we can use it as well to pretend we've consumed the result from such occasional syscalls and even remove the shut_* macro. Cheers, Willy
Re: [PATCH] BUILD: Avoid warning about ignoring write()'s return value in BUG_ON()
Willy, Am 13.03.20 um 17:34 schrieb Willy Tarreau: > Indeed, just found it in my queue. However we usually use it > differently, with the function instead of the variable. Do you > mind if I adapt it ? > I attempted to use the function, but it didn't compile. I guess because of a circular dependency. If it's a small obvious change feel free to adapt the patch. If it requires larger changes please fix it yourself and ignore my patch. Best regards Tim Düsterhus
Re: [PATCH] BUILD: Avoid warning about ignoring write()'s return value in BUG_ON()
On Fri, Mar 13, 2020 at 12:23:26PM +0100, Tim Düsterhus wrote: > Willy, > > Am 09.03.20 um 17:05 schrieb Tim Duesterhus: > > gcc complains (non-rightfully): > > > >> include/common/buf.h: In function 'br_head_pick': > >> include/common/debug.h:62:4: warning: ignoring return value of 'write', > >> declared with attribute warn_unused_result [-Wunused-result] > >> (void)write(2, msg, strlen(msg)); \ > >> ^ > >> include/common/debug.h:57:35: note: in expansion of macro '__BUG_ON' > >> #define _BUG_ON(cond, file, line) __BUG_ON(cond, file, line) > >>^ > >> include/common/debug.h:56:22: note: in expansion of macro '_BUG_ON' > >> #define BUG_ON(cond) _BUG_ON(cond, __FILE__, __LINE__) > >> ^ > >> include/common/buf.h:1011:2: note: in expansion of macro 'BUG_ON' > >> BUG_ON(r->area != BUF_RING.area); > >> ^ > > While checking my list of outgoing patches I noticed that this one > wasn't acknowledged yet. It will become important with: > https://github.com/haproxy/haproxy/issues/546 Indeed, just found it in my queue. However we usually use it differently, with the function instead of the variable. Do you mind if I adapt it ? Willy
Re: [PATCH] BUILD: Avoid warning about ignoring write()'s return value in BUG_ON()
Willy, Am 09.03.20 um 17:05 schrieb Tim Duesterhus: > gcc complains (non-rightfully): > >> include/common/buf.h: In function ‘br_head_pick’: >> include/common/debug.h:62:4: warning: ignoring return value of ‘write’, >> declared with attribute warn_unused_result [-Wunused-result] >> (void)write(2, msg, strlen(msg)); \ >> ^ >> include/common/debug.h:57:35: note: in expansion of macro ‘__BUG_ON’ >> #define _BUG_ON(cond, file, line) __BUG_ON(cond, file, line) >>^ >> include/common/debug.h:56:22: note: in expansion of macro ‘_BUG_ON’ >> #define BUG_ON(cond) _BUG_ON(cond, __FILE__, __LINE__) >> ^ >> include/common/buf.h:1011:2: note: in expansion of macro ‘BUG_ON’ >> BUG_ON(r->area != BUF_RING.area); >> ^ While checking my list of outgoing patches I noticed that this one wasn't acknowledged yet. It will become important with: https://github.com/haproxy/haproxy/issues/546 Best regards Tim Düsterhus
[PATCH] BUILD: Avoid warning about ignoring write()'s return value in BUG_ON()
gcc complains (non-rightfully):
> include/common/buf.h: In function ‘br_head_pick’:
> include/common/debug.h:62:4: warning: ignoring return value of ‘write’,
> declared with attribute warn_unused_result [-Wunused-result]
> (void)write(2, msg, strlen(msg)); \
> ^
> include/common/debug.h:57:35: note: in expansion of macro ‘__BUG_ON’
> #define _BUG_ON(cond, file, line) __BUG_ON(cond, file, line)
>^
> include/common/debug.h:56:22: note: in expansion of macro ‘_BUG_ON’
> #define BUG_ON(cond) _BUG_ON(cond, __FILE__, __LINE__)
> ^
> include/common/buf.h:1011:2: note: in expansion of macro ‘BUG_ON’
> BUG_ON(r->area != BUF_RING.area);
> ^
---
include/common/debug.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/common/debug.h b/include/common/debug.h
index df8552229..e748c9697 100644
--- a/include/common/debug.h
+++ b/include/common/debug.h
@@ -53,13 +53,14 @@
#define CRASH_NOW()
#endif
+extern int shut_your_big_mouth_gcc_int;
#define BUG_ON(cond) _BUG_ON(cond, __FILE__, __LINE__)
#define _BUG_ON(cond, file, line) __BUG_ON(cond, file, line)
#define __BUG_ON(cond, file, line)
\
do { \
if (unlikely(cond)) { \
const char msg[] = "\nFATAL: bug condition \"" #cond
"\" matched at " file ":" #line "\n"; \
- (void)write(2, msg, strlen(msg)); \
+ shut_your_big_mouth_gcc_int = write(2, msg,
strlen(msg)); \
CRASH_NOW(); \
} \
} while (0)
--
2.25.1
