Bug 155992 – Implement logging for RELEASE_BASSERT_WITH_MESSAGE() in BAssert.h

• Component: bmalloc
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

For Bug 155409, I added RELEASE_BASSERT_WITH_MESSAGE() in BAssert.h, but it doesn't actually log anything.

This bug is to implement logging for RELEASE_BASSERT_WITH_MESSAGE() in BAssert.h.

[David Kilzer (:ddkilzer)]

Created attachment 275273 [details]
Patch v1

[WebKit Commit Bot]

Attachment 275273 [details] did not pass style-queue:


ERROR: Source/bmalloc/bmalloc/BAssert.h:79:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 275273 [details]
Patch v1

r=me

I guess this is OK.

Do we need this? It looks like we'll only use it for soft linking failures. Are we suffering from soft linking failures we need to diagnose?

A problem with heavy and complex ASSERT macros is that they destroy the CPU context at the point of assertion failure, so they make crash reports much less useful.

So, I guess I can say OK to RELEASE_BASSERT_WITH_MESSAGE for soft linking because it's only a small part of the project, but this is an idiom I'd like to avoid using very much.

[David Kilzer (:ddkilzer)]

Created attachment 275505 [details]
Patch v2

[David Kilzer (:ddkilzer)]

(In reply to comment #3)
> Comment on attachment 275273 [details]
> Patch v1
> 
> r=me
> 
> I guess this is OK.
> 
> Do we need this? It looks like we'll only use it for soft linking failures.
> Are we suffering from soft linking failures we need to diagnose?
> 
> A problem with heavy and complex ASSERT macros is that they destroy the CPU
> context at the point of assertion failure, so they make crash reports much
> less useful.
> 
> So, I guess I can say OK to RELEASE_BASSERT_WITH_MESSAGE for soft linking
> because it's only a small part of the project, but this is an idiom I'd like
> to avoid using very much.

Patch v2 only does the extra logging in Debug builds, so Release builds won't destroy CPU context.

[WebKit Commit Bot]

Attachment 275505 [details] did not pass style-queue:


ERROR: Source/bmalloc/bmalloc/BAssert.h:82:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 275505 [details]
Patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=275505&action=review

r=me

> Source/bmalloc/bmalloc/BAssert.h:72
> +#define BMALLOC_LOGGING_PREFIX "#BM: "

Might as well throw in the full "bmalloc:" here if we can.

[David Kilzer (:ddkilzer)]

Comment on attachment 275505 [details]
Patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=275505&action=review

>> Source/bmalloc/bmalloc/BAssert.h:72
>> +#define BMALLOC_LOGGING_PREFIX "#BM: "
> 
> Might as well throw in the full "bmalloc:" here if we can.

I'll change this.  I was mimicking what was done in Source/WTF/wtf/Assertions.h:

#define WTF_LOGGING_PREFIX "#WK: "

[David Kilzer (:ddkilzer)]

Committed r201969: <http://trac.webkit.org/changeset/201969>