Skip to contentSkip to conversation

v2 [PATCH 2/2] sanity.bbclass: Implement initial toolchain sanity checks

PATCH Peter Seebach 2 participants 3 messages Apr 28, 2012 – Apr 30, 2012 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

(Fixed copy-and-paste error in documentation.conf.)

This introduces a sanity check for the toolchain, which verifies each tuning (including any multilibs), producing meaningful diagnostics for problems, and also provides some higher-level tuning features.

The TUNEVALID and TUNECONFLICT/TUNECONFLICTS settings were not implemented, and there were some loose ends (like not knowing how the conflict one was spelled). Listed one or two missing features in TUNEVALID, also (in a previous patch) fixed the references to features which didn't exist.

This patch also provides a whitelisting mechanism (which is completely unused) to allow vendors providing prebuilt toolchain components to restrict tunings to those based on or compatible with a particular ABI.

---
 meta/classes/sanity.bbclass                      |   69 ++++++++++++++++++++++
 meta/conf/documentation.conf                     |    6 ++
 meta/conf/machine/include/README                 |    4 +
 meta/conf/machine/include/arm/arch-armv5-dsp.inc |    1 +
 meta/conf/machine/include/arm/arch-armv7a.inc    |    2 +-
 meta/conf/machine/include/ia32/arch-ia32.inc     |    2 +-
 meta/conf/machine/include/mips/arch-mips.inc     |    6 +-
 meta/conf/machine/include/tune-c3.inc            |    2 +-
 8 files changed, 86 insertions(+), 6 deletions(-)
meta/classes/sanity.bbclass +69 -0
index 687ddeb..ff11fec 100644
@@ -11,6 +11,72 @@ def raise_sanity_error(msg):
     
     %s""" % msg)
 
+# Check a single tune for validity.
+def check_toolchain_tune(data, tune, multilib):
+    tune_errors = []
+    if not tune or tune == "":
+        return "No tuning found for %s multilib." % multilib
+    bb.note("Sanity-checking tuning '%s' (%s) features:" % (tune, multilib))
+    features = data.getVar("TUNE_FEATURES_tune-%s" % tune, True) or ""
+    if features == '':
+        return "Tuning '%s' has no defined features, and cannot be used." % tune
+    features = features.split()
+    valid_tunes = data.getVarFlags('TUNEVALID') or ""
+    conflicts = data.getVarFlags('TUNECONFLICTS') or ""
+    split_conflicts = {}
+    for feature in features:
+        if feature in conflicts:
+            if feature not in split_conflicts:
+                split_conflicts[feature] = conflicts[feature].split()
+            for other in features:
+                if other in split_conflicts[feature]:
+                    tune_errors.append("Feature '%s' conflicts with '%s'." %
+                        ( feature, other ))
+        if feature in valid_tunes:
+            bb.note("  %s: %s" % (feature, valid_tunes[feature]))
+        else:
+            tune_errors.append("Feature '%s' is not defined." % feature)
+    whitelist = data.getVar("TUNEABI_WHITELIST", True) or ''
+    if whitelist != '':
+        override = data.getVar("TUNEABI_OVERRIDE", True) or ''
+        if not override:
+            tuneabi = data.getVar("TUNEABI_tune-%s" % tune, True) or ""
+            if tuneabi == "":
+                tuneabi = tune
+            if True not in [x in whitelist.split() for x in tuneabi.split()]:
+                tune_errors.append("Tuning '%s' (%s) cannot be used with any supported tuning/ABI." %
+                    (tune, tuneabi))
+    if tune_errors:
+        return "Tuning '%s' has the following errors:\n" + '\n'.join(tune_errors)
+
+def check_toolchain(data):
+    tune_error_set = []
+    deftune = data.getVar("DEFAULTTUNE", True)
+    tune_errors = check_toolchain_tune(data, deftune, 'default')
+    if tune_errors:
+        tune_error_set.append(tune_errors)
+
+    multilibs = data.getVar("MULTILIBS", True) or ""
+    if multilibs != "":
+        for pairs in [x.split(':') for x in multilibs.split()]:
+            if pairs[0] != 'multilib':
+                bb.warn("Got an unexpected '%s' in MULTILIBS." % pairs[0])
+            else:
+                if pairs[1] == 'lib':
+                    tune_error_set.append("The multilib 'lib' was specified, but that causes parse errors.")
+                else:
+                    tune = data.getVar("DEFAULTTUNE_virtclass-multilib-%s" % pairs[1], True)
+                    if tune == deftune:
+                        bb.warn("Multilib '%s' (%s) is also the default tuning." % (pairs[1], deftune))
+                    else:
+                        tune_errors = check_toolchain_tune(data, tune, pairs[1])
+                    if tune_errors:
+                        tune_error_set.append(tune_errors)
+    if tune_error_set:
+        return "Toolchain tunings invalid:\n" + '\n'.join(tune_error_set)
+
+    return ""
+
 def check_conf_exists(fn, data):
     bbpath = []
     fn = data.expand(fn)
@@ -327,6 +393,9 @@ def check_sanity(e):
         messages = messages + pseudo_msg + '\n'
 
     check_supported_distro(e)
+    toolchain_msg = check_toolchain(e.data)
+    if toolchain_msg != "":
+        messages = messages + toolchain_msg + '\n'
 
     # Check if DISPLAY is set if IMAGETEST is set
     if not data.getVar( 'DISPLAY', e.data, True ) and data.getVar( 'IMAGETEST', e.data, True ) == 'qemu':
meta/conf/documentation.conf +6 -0
index f4d6241..99c16fb 100644
@@ -34,6 +34,12 @@ TARGET_CC_ARCH[doc] = "FIXME"
 TARGET_FPU[doc] = "Floating point option (mostly for FPU-less systems), can be 'soft' or empty \
 for hardware floating point instructions."
 
+# WARNING:  Because the flags on these have semantic implecations,
+# they must not actually be defined.
+#TUNEVALID[doc] = "Descriptions of valid tuning features, stored as flags."
+#TUNECONFLICTS[doc] = "List of conflicting features for a given feature."
+TUNEABI[doc] = "A base ABI used by a given tuning, used with prebuilt binaries."
+
 ASSUME_PROVIDED[doc] = "List of packages (recipes actually) which are assumed to be implicitly available.\
  These packages won't be built by bitbake."
 ASSUME_SHLIBS[doc] = "List of shlib:package[_version] mappings. Useful for lib packages in ASSUME_PROVIDED,\
meta/conf/machine/include/README +4 -0
index 6a3a63d..e4b59c9 100644
@@ -24,6 +24,10 @@ TUNEVALID[feature] - The <feature> is defined with a human readable
 explanation for what it does.  All architectural, cpu, abi, etc tuning 
 features must be defined using TUNEVALID.
 
+TUNECONFLICTS[feature] - A list of features which conflict with <feature>.
+New sanity checks will try to reject combinations in which a single
+tuning ends up with features which conflict with each other.
+
 TUNE_FEATURES - This is automatically defined as TUNE_FEATURES_tune-<tune>.
 See TUNE_FEATURES_tune-<tune> for more information.
 
meta/conf/machine/include/arm/arch-armv5-dsp.inc +1 -0
index 9f03a0f..0f64562 100644
@@ -1,4 +1,5 @@
 ARMPKGSFX_DSP = "${@bb.utils.contains("TUNE_FEATURES", [ "armv5", "dsp" ], "e", "", d)}"
+TUNEVALID[dsp] = "ARM DSP functionality"
 
 require conf/machine/include/arm/arch-armv5.inc
 
meta/conf/machine/include/arm/arch-armv7a.inc +1 -1
index 629960d..c90aff5 100644
@@ -2,7 +2,7 @@ DEFAULTTUNE ?= "armv7a"
 
 ARMPKGARCH ?= "armv7a"
 
-TUNEVALID[armv7-a] = "Enable instructions for ARMv7-a"
+TUNEVALID[armv7a] = "Enable instructions for ARMv7-a"
 TUNE_CONFLICTS[armv7a] = "armv4 armv5 armv6 armv7"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "armv7a", "-march=armv7-a -fno-tree-vectorize", "", d)}"
 MACHINEOVERRIDES .= "${@bb.utils.contains("TUNE_FEATURES", "armv7a", ":armv7a", "" ,d)}"
meta/conf/machine/include/ia32/arch-ia32.inc +1 -1
index a5dae88..15f67d7 100644
@@ -27,7 +27,7 @@ TUNE_ASARGS += "${@bb.utils.contains("TUNE_FEATURES", "mx32", "-x32", "", d)}"
 
 # ELF64 ABI
 TUNEVALID[m64] = "IA32e (x86_64) ELF64 standard ABI"
-TUNECONFLICT[m64] = "m32 mx32"
+TUNECONFLICTS[m64] = "m32 mx32"
 TUNE_ARCH .= "${@bb.utils.contains("TUNE_FEATURES", "m64", "${X86ARCH64}", "" ,d)}"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "m64", "-m64", "", d)}"
 
meta/conf/machine/include/mips/arch-mips.inc +3 -3
index 8758ecd..9f12920 100644
@@ -12,15 +12,15 @@ TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "bigendian", "-meb", "-mel
 
 # ABI flags
 TUNEVALID[o32] = "MIPS o32 ABI"
-TUNECONFLICT[o32] = "n32 n64"
+TUNECONFLICTS[o32] = "n32 n64"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "o32", "-mabi=32", "", d)}"
 
 TUNEVALID[n32] = "MIPS64 n32 ABI"
-TUNECONFLICT[n32] = "o32 n64"
+TUNECONFLICTS[n32] = "o32 n64"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "n32", "-mabi=n32", "", d)}"
 
 TUNEVALID[n64] = "MIPS64 n64 ABI"
-TUNECONFLICT[n64] = "o32 n32"
+TUNECONFLICTS[n64] = "o32 n32"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "n64", "-mabi=64", "", d)}"
 
 # Floating point
meta/conf/machine/include/tune-c3.inc +1 -1
index 06fac8f..79bb67b 100644
@@ -1,7 +1,7 @@
 require conf/machine/include/ia32/arch-ia32.inc
 
 TUNEVALID[c3] = "VIA Cyrix III or VIA C3 specific optimizations"
-TUNECONFLICT[c3] = "m64 mx32"
+TUNECONFLICTS[c3] = "m64 mx32"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "c3", "-march=c3 -mtune=c3", "", d)}"
 
 AVAILTUNES += "c3"
Signature · 1 line
1.7.0.4
#2 lore raw
Quoted 7 lines · Peter Seebach

+    valid_tunes = data.getVarFlags('TUNEVALID') or ""
+    conflicts = data.getVarFlags('TUNECONFLICTS') or ""
+    split_conflicts = {}
+    for feature in features:
+        if feature in conflicts:
+            if feature not in split_conflicts:
+                split_conflicts[feature] = conflicts[feature].split()

Quoted 8 lines

+            for other in features:
+                if other in split_conflicts[feature]:
+                    tune_errors.append("Feature '%s' conflicts with '%s'." %
+                        ( feature, other ))
+        if feature in valid_tunes:
+            bb.note("  %s: %s" % (feature, valid_tunes[feature]))
+        else:
+            tune_errors.append("Feature '%s' is not defined." % feature)

can this piece be simplified. ? may be using sets or some other python contructs I think its quite difficult to comprehend for me whats going on here

#3 lore raw
Quoted 10 lines · On Sat, 28 Apr 2012 16:42:31 -0700 Khem Raj <[email protected]> wrote:
Quoted 10 lines

+    valid_tunes = data.getVarFlags('TUNEVALID') or ""
+    conflicts = data.getVarFlags('TUNECONFLICTS') or ""
+    split_conflicts = {}
+    for feature in features:
+        if feature in conflicts:
+            if feature not in split_conflicts:
+                split_conflicts[feature] = conflicts[feature].split()
+            for other in features:
+                if other in split_conflicts[feature]:

Quoted 8 lines
Quoted 8 lines

+                    tune_errors.append("Feature '%s' conflicts with '%s'." %
+                        ( feature, other ))
+        if feature in valid_tunes:
+            bb.note("  %s: %s" % (feature, valid_tunes[feature]))
+        else:
+            tune_errors.append("Feature '%s' is not defined." % feature)

Using sets seems a little like overkill, because I really do want to

Quoted 7 lines · check specifically which features are in conflict. That
Quoted 7 lines

+    valid_tunes = data.getVarFlags('TUNEVALID') or ""
+    conflicts = data.getVarFlags('TUNECONFLICTS') or ""
+    split_conflicts = {}
+    for feature in features:
+        if feature in conflicts:
+            for conflict in conflicts[feature].split():
+                if conflict in features:

Quoted 8 lines
Quoted 8 lines

+                    tune_errors.append("Feature '%s' conflicts with '%s'." %
+                        ( feature, conflict ))
+        if feature in valid_tunes:
+            bb.note("  %s: %s" % (feature, valid_tunes[feature]))
+        else:
+            tune_errors.append("Feature '%s' is not defined." % feature)

I can drop the "optimization" of caching the split results, since I believe most features only have one conflict anyway, and Python may well be optimizing it for me.

Basically, the first part is "for each feature, if it has any conflicts,check them against all the other features."

My original thought had been to write this the way I did the whitelist, something like:

if True in [x in features for x in conflicts[feature].split()]

but this wouldn't let me diagnose which feature conflicted. So when I started on this, I was thinking that I had to know which feature I'd found in the conflicts list, but of course, the entries in the conflicts list *are* the features. So I could simplify it as above.

-s

Signature · 1 line
Listen, get this.  Nobody with a good compiler needs to be justified.