Bug 171575

Summary: Add a debugging macro that sleeps a thread until a debugger attaches
Product: WebKit Reporter: BJ Burg <bburg>
Component: Web Template FrameworkAssignee: BJ Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, benjamin, buildbot, cdumez, cmarcelo, dbates, joepeck, mark.lam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
For Landing none

Description BJ Burg 2017-05-02 14:48:50 PDT 
I use this all the time, might as well check it in.
Comment 1 BJ Burg 2017-05-03 09:56:39 PDT 
Created attachment 308917 [details]
Patch
Comment 2 BJ Burg 2017-05-03 10:03:52 PDT 
Created attachment 308919 [details]
Patch
Comment 3 Build Bot 2017-05-03 10:04:36 PDT 
Attachment 308919 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/DebugUtilities.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Mark Lam 2017-05-03 11:03:37 PDT 
Comment on attachment 308919 [details]
Patch

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

> Source/WTF/wtf/Assertions.h:197
> -#if defined(NDEBUG) && OS(DARWIN)
>  #if CPU(X86_64) || CPU(X86)
>  #define WTFBreakpointTrap()  __asm__ volatile ("int3")
>  #elif CPU(ARM_THUMB2)
>  #define WTFBreakpointTrap()  __asm__ volatile ("bkpt #0")
>  #elif CPU(ARM64)
>  #define WTFBreakpointTrap()  __asm__ volatile ("brk #0")

I've only validated these asm statements on OS(DARWIN).  Are you sure they work for non-Darwin ports?
Comment 5 BJ Burg 2017-05-03 11:43:56 PDT 
Created attachment 308932 [details]
Patch
Comment 6 Build Bot 2017-05-03 11:45:21 PDT 
Attachment 308932 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/DebugUtilities.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 BJ Burg 2017-05-03 11:54:44 PDT 
(In reply to Mark Lam from comment #4)
> Comment on attachment 308919 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=308919&action=review
> 
> > Source/WTF/wtf/Assertions.h:197
> > -#if defined(NDEBUG) && OS(DARWIN)
> >  #if CPU(X86_64) || CPU(X86)
> >  #define WTFBreakpointTrap()  __asm__ volatile ("int3")
> >  #elif CPU(ARM_THUMB2)
> >  #define WTFBreakpointTrap()  __asm__ volatile ("bkpt #0")
> >  #elif CPU(ARM64)
> >  #define WTFBreakpointTrap()  __asm__ volatile ("brk #0")
> 
> I've only validated these asm statements on OS(DARWIN).  Are you sure they
> work for non-Darwin ports?

I am unsure how they would differ. Software breakpoint instructions are a detail of the ISA, not the OS. Right?
Comment 8 Mark Lam 2017-05-03 12:00:58 PDT 
(In reply to Brian Burg from comment #7)
> (In reply to Mark Lam from comment #4)
> > Comment on attachment 308919 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=308919&action=review
> > 
> > > Source/WTF/wtf/Assertions.h:197
> > > -#if defined(NDEBUG) && OS(DARWIN)
> > >  #if CPU(X86_64) || CPU(X86)
> > >  #define WTFBreakpointTrap()  __asm__ volatile ("int3")
> > >  #elif CPU(ARM_THUMB2)
> > >  #define WTFBreakpointTrap()  __asm__ volatile ("bkpt #0")
> > >  #elif CPU(ARM64)
> > >  #define WTFBreakpointTrap()  __asm__ volatile ("brk #0")
> > 
> > I've only validated these asm statements on OS(DARWIN).  Are you sure they
> > work for non-Darwin ports?
> 
> I am unsure how they would differ. Software breakpoint instructions are a
> detail of the ISA, not the OS. Right?

Yes, but inline asm statement syntax?
Comment 9 BJ Burg 2017-05-03 12:24:23 PDT 
(In reply to Mark Lam from comment #8)
> (In reply to Brian Burg from comment #7)
> > (In reply to Mark Lam from comment #4)
> > > Comment on attachment 308919 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=308919&action=review
> > > 
> > > > Source/WTF/wtf/Assertions.h:197
> > > > -#if defined(NDEBUG) && OS(DARWIN)
> > > >  #if CPU(X86_64) || CPU(X86)
> > > >  #define WTFBreakpointTrap()  __asm__ volatile ("int3")
> > > >  #elif CPU(ARM_THUMB2)
> > > >  #define WTFBreakpointTrap()  __asm__ volatile ("bkpt #0")
> > > >  #elif CPU(ARM64)
> > > >  #define WTFBreakpointTrap()  __asm__ volatile ("brk #0")
> > > 
> > > I've only validated these asm statements on OS(DARWIN).  Are you sure they
> > > work for non-Darwin ports?
> > 
> > I am unsure how they would differ. Software breakpoint instructions are a
> > detail of the ISA, not the OS. Right?
> 
> Yes, but inline asm statement syntax?

Oh, yeah, I have no idea. We seem to use the same macros in bmalloc (BAssert.h) and those are not specific to OS(DARWIN). Some of the 3rd-party libraries we use for WebRTC also use __asm__ volatile ("...") syntax. My understanding is that those build for GTK port as well.

In any case, this patch does not add new clients of these macros. If it is indeed broken with another compiler – something I can't verify myself – then the macros can be generalized in a later patch if it does not fail EWS.
Comment 10 Mark Lam 2017-05-04 10:22:55 PDT 
Comment on attachment 308932 [details]
Patch

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

r=me with suggested improvement.

> Source/WTF/wtf/Assertions.h:202
> +#define WTFBreakpointTrap()

Hmmm.  If we want to make WTFBreakpointTrap() an API for more general use, I question whether it's wise to allow it to fail silently like this if it's not defined.  Instead, how about defining it as:

#define WTFBreakpointTrap() WTFCrash() // Not implemented

If someone tries to use it, they will crash and the trail will lead to the "Not implemented" comment.
Comment 11 BJ Burg 2017-05-04 11:12:20 PDT 
(In reply to Mark Lam from comment #10)
> Comment on attachment 308932 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=308932&action=review
> 
> r=me with suggested improvement.
> 
> > Source/WTF/wtf/Assertions.h:202
> > +#define WTFBreakpointTrap()
> 
> Hmmm.  If we want to make WTFBreakpointTrap() an API for more general use, I
> question whether it's wise to allow it to fail silently like this if it's
> not defined.  Instead, how about defining it as:
> 
> #define WTFBreakpointTrap() WTFCrash() // Not implemented
> 
> If someone tries to use it, they will crash and the trail will lead to the
> "Not implemented" comment.

OK
Comment 12 BJ Burg 2017-05-04 12:56:58 PDT 
Created attachment 309093 [details]
For Landing
Comment 13 Build Bot 2017-05-04 12:58:13 PDT 
Attachment 309093 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/DebugUtilities.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 BJ Burg 2017-05-22 11:49:09 PDT 
Committed r217233: <http://trac.webkit.org/changeset/217233>
Comment 15 BJ Burg 2017-05-22 11:49:28 PDT 
Comment on attachment 309093 [details]
For Landing

Landed manually.