Bug 157364 – Enable Dwarf2 debug information in offline assembler for clang compiler

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

Change set r196541 added the output of Dwarf2 debugging directives to the offline assembler, but disabled the feature until the appropriate support was added to the C++ compiler.  That support was added to Clang version 800.0.12 and later.

We should detect that we're compiling with the appropriate clang compiler version and enable the output of the Dwarf2 directives.

[Radar WebKit Bug Importer]

<rdar://problem/26100879>

[Michael Saboff]

Created attachment 278143 [details]
Patch

[Mark Lam]

Comment on attachment 278143 [details]
Patch

rs=me

[WebKit Commit Bot]

Comment on attachment 278143 [details]
Patch

Clearing flags on attachment: 278143

Committed r200447: <http://trac.webkit.org/changeset/200447>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

Comment on attachment 278143 [details]
Patch

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

> Source/JavaScriptCore/offlineasm/config.rb:66
> +        clangExecutable = ENV['TOOLCHAIN_DIR'] + '/usr/bin/clang'
> +        if File.executable?(clangExecutable)
> +            clangVersionOut = %x`#{clangExecutable} --version`

The canonical way to find and invoke clang from a script running in Xcode would be simply `xcrun clang --version`

[Csaba Osztrogonác]

(In reply to comment #4)
> Comment on attachment 278143 [details]
> Patch
> 
> Clearing flags on attachment: 278143
> 
> Committed r200447: <http://trac.webkit.org/changeset/200447>

It broke the Apple Mac Yosemite build:
(see build.webkit.org for details)

/Volumes/Data/slave/yosemite-release/build/Source/JavaScriptCore/offlineasm/config.rb:64:in `shouldEnableDebugAnnotations': undefined method `+' for nil:NilClass (NoMethodError)
	from /Volumes/Data/slave/yosemite-release/build/Source/JavaScriptCore/offlineasm/config.rb:83:in `<top (required)>'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /Volumes/Data/slave/yosemite-release/build/Source/JavaScriptCore/offlineasm/generate_offset_extractor.rb:28:in `<main>'
Command /bin/sh failed with exit code 1

[Csaba Osztrogonác]

Comment on attachment 278143 [details]
Patch

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

> Source/JavaScriptCore/offlineasm/config.rb:74
> +                # clang version 800.0.12 or higher is required for debug annotations
> +                versionMatch = /clang-(\d+).(\d+).(\d+)/.match(clangVersionOut)
> +                if versionMatch.length >= 4
> +                    totalVersion = versionMatch[1].to_i * 1000000 + versionMatch[2].to_i * 1000 + versionMatch[3].to_i
> +                    if totalVersion >= 800000012
> +                        return true
> +                    end

It is not platform independent at all, it is a Mac only code.
There is no GCC_VERSION, no TOOLCHAIN_DIR on non Mac platforms, 
no similar clang versions like this.

Some Linux examples for clang --version.

$ clang --version
Ubuntu clang version 3.4-1ubuntu3 (tags/RELEASE_34/final) (based on LLVM 3.4)
Target: x86_64-pc-linux-gnu
Thread model: posix

$ clang --version
Ubuntu clang version 3.6.2-1 (tags/RELEASE_362/final) (based on LLVM 3.6.2)
Target: x86_64-pc-linux-gnu
Thread model: posix

[Csaba Osztrogonác]

(In reply to comment #7)
> (In reply to comment #4)
> > Comment on attachment 278143 [details]
> > Patch
> > 
> > Clearing flags on attachment: 278143
> > 
> > Committed r200447: <http://trac.webkit.org/changeset/200447>
> 
> It broke the Apple Mac Yosemite build:
> (see build.webkit.org for details)
> 
> /Volumes/Data/slave/yosemite-release/build/Source/JavaScriptCore/offlineasm/
> config.rb:64:in `shouldEnableDebugAnnotations': undefined method `+' for
> nil:NilClass (NoMethodError)
> 	from
> /Volumes/Data/slave/yosemite-release/build/Source/JavaScriptCore/offlineasm/
> config.rb:83:in `<top (required)>'
> 	from
> /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/
> rubygems/core_ext/kernel_require.rb:55:in `require'
> 	from
> /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/
> rubygems/core_ext/kernel_require.rb:55:in `require'
> 	from
> /Volumes/Data/slave/yosemite-release/build/Source/JavaScriptCore/offlineasm/
> generate_offset_extractor.rb:28:in `<main>'
> Command /bin/sh failed with exit code 1

After checking the Yosemite build logs, it seems 
there is no TOOLCHAIN_DIR env, only DT_TOOLCHAIN_DIR.

(In reply to comment #9)
> (In reply to comment #7)
> > (In reply to comment #4)
> > > Comment on attachment 278143 [details]
> > > Patch
> > > 
> > > Clearing flags on attachment: 278143
> > > 
> > > Committed r200447: <http://trac.webkit.org/changeset/200447>
> > 
> > It broke the Apple Mac Yosemite build:
> > (see build.webkit.org for details)
> > 
> > /Volumes/Data/slave/yosemite-release/build/Source/JavaScriptCore/offlineasm/
> > config.rb:64:in `shouldEnableDebugAnnotations': undefined method `+' for
> > nil:NilClass (NoMethodError)
> > 	from
> > /Volumes/Data/slave/yosemite-release/build/Source/JavaScriptCore/offlineasm/
> > config.rb:83:in `<top (required)>'
> > 	from
> > /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/
> > rubygems/core_ext/kernel_require.rb:55:in `require'
> > 	from
> > /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/
> > rubygems/core_ext/kernel_require.rb:55:in `require'
> > 	from
> > /Volumes/Data/slave/yosemite-release/build/Source/JavaScriptCore/offlineasm/
> > generate_offset_extractor.rb:28:in `<main>'
> > Command /bin/sh failed with exit code 1
> 
> After checking the Yosemite build logs, it seems 
> there is no TOOLCHAIN_DIR env, only DT_TOOLCHAIN_DIR.

Relying on either variable, or on the path to clang relative to it, isn’t the right thing to do. As I’ve mentioned above, xcrun is the right way to invoke clang. If this script is shared with non-Xcode build systems, then it’s probably best for the script itself to use the CC environment variable, and for the Xcode script build phase that invokes it to set that variable (if not already set) to the output of "xcrun -find clang".

[Michael Saboff]

Looking at the build breakage now.

[Michael Saboff]

Landed build fix in change set r200458: <http://trac.webkit.org/changeset/200458>

[Michael Saboff]

(In reply to comment #8)
> Comment on attachment 278143 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=278143&action=review
> 
> > Source/JavaScriptCore/offlineasm/config.rb:74
> > +                # clang version 800.0.12 or higher is required for debug annotations
> > +                versionMatch = /clang-(\d+).(\d+).(\d+)/.match(clangVersionOut)
> > +                if versionMatch.length >= 4
> > +                    totalVersion = versionMatch[1].to_i * 1000000 + versionMatch[2].to_i * 1000 + versionMatch[3].to_i
> > +                    if totalVersion >= 800000012
> > +                        return true
> > +                    end
> 
> It is not platform independent at all, it is a Mac only code.
> There is no GCC_VERSION, no TOOLCHAIN_DIR on non Mac platforms, 
> no similar clang versions like this.
> 
> Some Linux examples for clang --version.
> 
> $ clang --version
> Ubuntu clang version 3.4-1ubuntu3 (tags/RELEASE_34/final) (based on LLVM 3.4)
> Target: x86_64-pc-linux-gnu
> Thread model: posix
> 
> $ clang --version
> Ubuntu clang version 3.6.2-1 (tags/RELEASE_362/final) (based on LLVM 3.6.2)
> Target: x86_64-pc-linux-gnu
> Thread model: posix

I intended the change to be Mac only.  I have no way of determining what Linux clang versions have the multiple .file directives fix.

[Michael Saboff]

(In reply to comment #13)

> I intended the change to be Mac only.  I have no way of determining what
> Linux clang versions have the multiple .file directives fix.

Please add Linux clang specific check.  You'll need to figure out the first clang version that has the fix given at http://reviews.llvm.org/D16101.

[Alexey Proskuryakov]

Can this be detected by a feature check, instead of a version check?

It's not clear from the bug what the failure mode was.

[Michael Saboff]

(In reply to comment #15)
> Can this be detected by a feature check, instead of a version check?
> 
> It's not clear from the bug what the failure mode was.

Without the clang fix, clang will crash with the directives this change produces.  There isn't a feature check in clang for the change.