999893 - Also skip integrated-as when building with scan-build

Status: RESOLVED FIXED

(NSS :: Build, defect, P2)

Description

[Sylvestre Ledru [:Sylvestre]]

Attachment: scan-build.diff

scan-build is a front end to the clang analyzer. It is provided in the LLVM/Clang suite.
When building Firefox with scan-build "scan-build mach build", clang does not detect it as clang and still try to use "-no-integrated-as" which fails.
The attached patch also detects scan-build to remove the usage of this option.

Comment 1

[Wan-Teh Chang]

Comment on attachment 8410735 [details] [diff] [review]
scan-build.diff

Review of attachment 8410735 [details] [diff] [review]:
-----------------------------------------------------------------

Sylvestre:

Thank you for the patch. The original check is a hack. It's time to
write a proper check that parses the version string of the assembler.

::: lib/freebl/Makefile
@@ +663,5 @@
>  # expression of a .set directive. intel-gcm.s uses .set to give
>  # symbolic names to registers, for example,
>  #     .set  Htbl, %rdi
>  # So we can't use Clang's integrated assembler with intel-gcm.s.
> +ifneq (,$(filter clang scan-build,$(AS)))

What is the value of $(AS) for scan-build?

Can you run "$(AS) --version" and "$(AS) -v" manually and
paste the output here?

I will write the proper check for clang.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

AS = /usr/share/clang/scan-build/ccc-analyzer

By default, ccc-analyzer is connected to gcc:
$ /usr/share/clang/scan-build/ccc-analyzer --version
gcc (Debian 4.8.2-21) 4.8.2
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

scan-build will update it to run clang:
$ clang --version
Debian clang version 3.5.0-1~exp1 (trunk) (based on LLVM 3.5.0)
Target: x86_64-pc-linux-gnu
Thread model: posix

$ clang -v
Debian clang version 3.5.0-1~exp1 (trunk) (based on LLVM 3.5.0)
Target: x86_64-pc-linux-gnu
Thread model: posix
Found candidate GCC installation: /usr/bin/../lib/gcc/i486-linux-gnu/4.8
Found candidate GCC installation: /usr/bin/../lib/gcc/i486-linux-gnu/4.8.2
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/4.7
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/4.7.3
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/4.8
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/4.8.2
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9.0
Found candidate GCC installation: /usr/lib/gcc/i486-linux-gnu/4.8
Found candidate GCC installation: /usr/lib/gcc/i486-linux-gnu/4.8.2
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.7
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.7.3
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.8
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.8.2
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.9
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.9.0
Selected GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/4.8


Let me know if I can help.

Comment 3

[Wan-Teh Chang]

Sylvestre: thank you for the info. It shows "--version" is better
than "-v", whose output is too verbose.

The fix is to use the GNU make "shell" function to invoke

    $(AS) --version

and grep "clang" or "clang version" in the output. You can find
similar code in nss/coreconf/Darwin.mk (search for "-dumpversion")
and nss/coreconf/SunOS5.mk (search for "findstring GNU").

Something like this should work:

diff --git a/lib/freebl/Makefile b/lib/freebl/Makefile
--- a/lib/freebl/Makefile
+++ b/lib/freebl/Makefile
@@ -645,12 +645,13 @@ ifdef INTEL_GCM
 #
 $(OBJDIR)/$(PROG_PREFIX)intel-gcm-wrap$(OBJ_SUFFIX): CFLAGS += -mssse3
 
 # The integrated assembler in Clang 3.2 does not support % in the
 # expression of a .set directive. intel-gcm.s uses .set to give
 # symbolic names to registers, for example,
 #     .set  Htbl, %rdi
 # So we can't use Clang's integrated assembler with intel-gcm.s.
-ifneq (,$(findstring clang,$(AS)))
+CLANG_VERSION_STR = clang version
+ifneq (,$(findstring $(CLANG_VERSION_STR),$(shell $(CC) --version)))
 $(OBJDIR)/$(PROG_PREFIX)intel-gcm$(OBJ_SUFFIX): ASFLAGS += -no-integrated-as
 endif
 endif

Comment 4

[Sylvestre Ledru [:Sylvestre]]

I confirm that your fix is working. Thanks.

Comment 5

[Wan-Teh Chang]

Attachment: Search for "clang version" in the output of $(CC) --version to detect clang

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8422100 [details] [diff] [review]
Search for "clang version" in the output of $(CC) --version to detect clang

Review of attachment 8422100 [details] [diff] [review]:
-----------------------------------------------------------------

::: lib/freebl/Makefile
@@ +664,5 @@
>  # symbolic names to registers, for example,
>  #     .set  Htbl, %rdi
>  # So we can't use Clang's integrated assembler with intel-gcm.s.
> +CLANG_VERSION_STR = clang version
> +ifneq (,$(findstring $(CLANG_VERSION_STR),$(shell $(CC) --version)))

A cursory google search suggests that at least some versions of clang on mac don't return a string with "clang version" in it.

Like:
Apple LLVM version 5.0 (clang-500.2.79) (based on LLVM 3.3svn)
Target: x86_64-apple-darwin12.5.0
Thread model: posix

Comment 7

[Wan-Teh Chang]

Attachment: Search for "clang" in the output of $(CC) --version to detect clang

Thanks. I was worried that "clang" may occur as part
of some other word in the version string. But I think
that is unlikely. I guess we could also search for
"LLVM".

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8422128 [details] [diff] [review]
Search for "clang" in the output of $(CC) --version to detect clang

Review of attachment 8422128 [details] [diff] [review]:
-----------------------------------------------------------------

I think searching for clang is fine. LLVM carries the risk of hitting a non-clang llvm-based compiler (I think intel has one, now)

Comment 9

[Wan-Teh Chang]

Attachment: Search for "clang" in the output of $(AS) --version to detect clang

I just realized that we should check $(AS) --version rather
than $(CC) --version. (By default AS is set to $(CC), so it
doesn't really matter in this case.) Sorry about the mistake.

Comment 10

[Wan-Teh Chang]

Comment on attachment 8422144 [details] [diff] [review]
Search for "clang" in the output of $(AS) --version to detect clang

Patch checked in:
https://hg.mozilla.org/projects/nss/rev/0dbd1a83bc88
https://hg.mozilla.org/projects/nss/rev/9fb676267225

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Thanks!

Comment 12

[Wan-Teh Chang]

Comment on attachment 8422144 [details] [diff] [review]
Search for "clang" in the output of $(AS) --version to detect clang

Patch pushed to mozilla-central as part of NSS_3_16_2_BETA2:
https://hg.mozilla.org/mozilla-central/rev/980e7d78b358