[WebAssembly] Improve feature validation error messages

[tlively]

Summary

Add the names of the input files responsible for each error to the
messages.

[sbc100]

lld/wasm/Writer.cpp

379 ↗

(On Diff #202301)

So in the case the multiple files use a given feature it will show just one them? (i.e. this insert fails after the first one?)

419 ↗

(On Diff #202301)

Why not StringRef?

[tlively]

• Use StringRefs

[tlively]

lld/wasm/Writer.cpp

379 ↗

(On Diff #202301)

That's correct. I figured that printing all relevant files could get overwhelming rather quickly and that even when it would be reasonable, we still get 80% of the benefit by printing just one file.

419 ↗

(On Diff #202301)

Switched, and also above. I'm always wary about StringRef because I've been burned by it not owning its contents one too many times. I assume when it is used to insert something into a StringMap the string value is copied, so it should be ok here.

lld/trunk/test/wasm/shared-memory-no-atomics.yaml

Features:
         Name:          "atomics"
 ...
 
 # NO-SHARED:      - Type:            MEMORY
 # NO-SHARED-NEXT:   Memories:
 # NO-SHARED-NEXT:     - Initial:         0x00000002
 # NO-SHARED-NOT:        Maximum:
 
-# SHARED: 'atomics' feature is disallowed, so --shared-memory must not be used{{$}}
+# SHARED: 'atomics' feature is disallowed by {{.*}}shared-memory-no-atomics.yaml.tmp1.o, so --shared-memory must not be used{{$}}

lld/trunk/test/wasm/target-feature-required.yaml

 # SPECIFIED-NEXT:       - Prefix:          USED
 # SPECIFIED-NEXT:         Name:            bar
 # SPECIFIED-NEXT:       - Prefix:          USED
 # SPECIFIED-NEXT:         Name:            baz
 # SPECIFIED-NEXT:       - Prefix:          USED
 # SPECIFIED-NEXT:         Name:            foo
 # SPECIFIED-NEXT: ...
 
-# UNSPECIFIED: Target feature 'foo' is not allowed.{{$}}
+# UNSPECIFIED: Target feature 'foo' used by {{.*}}target-feature-required.yaml.tmp1.o is not allowed.{{$}}
 
 # UNSPECIFIED-NOCHECK:        - Type:            CUSTOM
 # UNSPECIFIED-NOCHECK-NEXT:     Name:            target_features
 # UNSPECIFIED-NOCHECK-NEXT:     Features:
 # UNSPECIFIED-NOCHECK-NEXT:       - Prefix:          USED
 # UNSPECIFIED-NOCHECK-NEXT:         Name:            bar
 # UNSPECIFIED-NOCHECK-NEXT:       - Prefix:          USED
 # UNSPECIFIED-NOCHECK-NEXT:         Name:            baz
 # UNSPECIFIED-NOCHECK-NEXT:       - Prefix:          USED
 # UNSPECIFIED-NOCHECK-NEXT:         Name:            quux
 # UNSPECIFIED-NOCHECK-NEXT: ...
 
 # REQUIRED:        - Type:            CUSTOM
 # REQUIRED-NEXT:     Name:            target_features
 # REQUIRED-NEXT:     Features:
 # REQUIRED-NEXT:       - Prefix:          USED
 # REQUIRED-NEXT:         Name:            foo
 # REQUIRED-NEXT: ...
 
-# DISALLOWED: Target feature 'foo' is disallowed. Use --no-check-features to suppress.{{$}}
+# DISALLOWED: Target feature 'foo' used in {{.*}}target-feature-required.yaml.tmp1.o is disallowed by {{.*}}target-feature-required.yaml.tmp.disallowed.o. Use --no-check-features to suppress.{{$}}
 
 # DISALLOWED-NOCHECK:        - Type:            CUSTOM
 # DISALLOWED-NOCHECK-NEXT:     Name:            target_features
 # DISALLOWED-NOCHECK-NEXT:     Features:
 # DISALLOWED-NOCHECK-NEXT:       - Prefix:          USED
 # DISALLOWED-NOCHECK-NEXT:         Name:            foo
 # DISALLOWED-NOCHECK-NEXT: ...
 
-# NONE: Missing required target feature 'foo'. Use --no-check-features to suppress.{{$}}
+# NONE: Missing target feature 'foo' in {{.*}}target-feature-required.yaml.tmp.none.o, required by {{.*}}target-feature-required.yaml.tmp1.o. Use --no-check-features to suppress.{{$}}
 
 # NONE-NOCHECK:        - Type:            CUSTOM
 # NONE-NOCHECK-NEXT:     Name:            target_features
 # NONE-NOCHECK-NEXT:     Features:
 # NONE-NOCHECK-NEXT:       - Prefix:          USED
 # NONE-NOCHECK-NEXT:         Name:            foo
 # NONE-NOCHECK-NEXT: ...

lld/trunk/test/wasm/target-feature-used.yaml

 # SPECIFIED-NEXT:       - Prefix:          USED
 # SPECIFIED-NEXT:         Name:            bar
 # SPECIFIED-NEXT:       - Prefix:          USED
 # SPECIFIED-NEXT:         Name:            baz
 # SPECIFIED-NEXT:       - Prefix:          USED
 # SPECIFIED-NEXT:         Name:            foo
 # SPECIFIED-NEXT: ...
 
-# UNSPECIFIED: Target feature 'foo' is not allowed.{{$}}
+# UNSPECIFIED: Target feature 'foo' used by {{.*}}target-feature-used.yaml.tmp1.o is not allowed.{{$}}
 
 # UNSPECIFIED-NOCHECK:        - Type:            CUSTOM
 # UNSPECIFIED-NOCHECK-NEXT:     Name:            target_features
 # UNSPECIFIED-NOCHECK-NEXT:     Features:
 # UNSPECIFIED-NOCHECK-NEXT:       - Prefix:          USED
 # UNSPECIFIED-NOCHECK-NEXT:         Name:            bar
 # UNSPECIFIED-NOCHECK-NEXT:       - Prefix:          USED
 # UNSPECIFIED-NOCHECK-NEXT:         Name:            baz

 
 # REQUIRED:        - Type:            CUSTOM
 # REQUIRED-NEXT:     Name:            target_features
 # REQUIRED-NEXT:     Features:
 # REQUIRED-NEXT:       - Prefix:          USED
 # REQUIRED-NEXT:         Name:            foo
 # REQUIRED-NEXT: ...
 
-# DISALLOWED: Target feature 'foo' is disallowed. Use --no-check-features to suppress.{{$}}
+# DISALLOWED: Target feature 'foo' used in {{.*}}target-feature-used.yaml.tmp1.o is disallowed by {{.*}}target-feature-used.yaml.tmp.disallowed.o. Use --no-check-features to suppress.{{$}}
 
 # DISALLOWED-NOCHECK:        - Type:            CUSTOM
 # DISALLOWED-NOCHECK-NEXT:     Name:            target_features
 # DISALLOWED-NOCHECK-NEXT:     Features:
 # DISALLOWED-NOCHECK-NEXT:       - Prefix:          USED
 # DISALLOWED-NOCHECK-NEXT:         Name:            foo
 # DISALLOWED-NOCHECK-NEXT: ...
 
 # NONE:        - Type:            CUSTOM
 # NONE-NEXT:     Name:            target_features
 # NONE-NEXT:     Features:
 # NONE-NEXT:       - Prefix:          USED
 # NONE-NEXT:         Name:            foo
 # NONE-NEXT: ...

lld/trunk/wasm/Writer.cpp

void Writer::finalizeSections() {
   for (OutputSection *S : OutputSections) {
     S->setOffset(FileSize);
     S->finalizeContents();
     FileSize += S->getSize();
   }
 }
 
 void Writer::populateTargetFeatures() {
-  SmallSet<std::string, 8> Used;
-  SmallSet<std::string, 8> Required;
-  SmallSet<std::string, 8> Disallowed;
+  StringMap<std::string> Used;
+  StringMap<std::string> Required;
+  StringMap<std::string> Disallowed;
 
   // Only infer used features if user did not specify features
   bool InferFeatures = !Config->Features.hasValue();
 
   if (!InferFeatures) {
     for (auto &Feature : Config->Features.getValue())
       Out.TargetFeaturesSec->Features.insert(Feature);
     // No need to read or check features
     if (!Config->CheckFeatures)
       return;
   }
 
   // Find the sets of used, required, and disallowed features
   for (ObjFile *File : Symtab->ObjectFiles) {
+    StringRef FileName(File->getName());
     for (auto &Feature : File->getWasmObj()->getTargetFeatures()) {
       switch (Feature.Prefix) {
       case WASM_FEATURE_PREFIX_USED:
-        Used.insert(Feature.Name);
+        Used.insert({Feature.Name, FileName});
         break;
       case WASM_FEATURE_PREFIX_REQUIRED:
-        Used.insert(Feature.Name);
-        Required.insert(Feature.Name);
+        Used.insert({Feature.Name, FileName});
+        Required.insert({Feature.Name, FileName});
         break;
       case WASM_FEATURE_PREFIX_DISALLOWED:
-        Disallowed.insert(Feature.Name);
+        Disallowed.insert({Feature.Name, FileName});
         break;
       default:
         error("Unrecognized feature policy prefix " +
               std::to_string(Feature.Prefix));
       }
     }
   }
 
   if (InferFeatures)
-    Out.TargetFeaturesSec->Features.insert(Used.begin(), Used.end());
+    Out.TargetFeaturesSec->Features.insert(Used.keys().begin(),
+                                           Used.keys().end());
 
-  if (Out.TargetFeaturesSec->Features.count("atomics") && !Config->SharedMemory)
+  if (Out.TargetFeaturesSec->Features.count("atomics") &&
+      !Config->SharedMemory) {
+    if (InferFeatures)
+      error(Twine("'atomics' feature is used by ") + Used["atomics"] +
+            ", so --shared-memory must be used");
+    else
       error("'atomics' feature is used, so --shared-memory must be used");
+  }
 
   if (!Config->CheckFeatures)
     return;
 
   if (Disallowed.count("atomics") && Config->SharedMemory)
-    error(
-        "'atomics' feature is disallowed, so --shared-memory must not be used");
+    error("'atomics' feature is disallowed by " + Disallowed["atomics"] +
+          ", so --shared-memory must not be used");
 
   // Validate that used features are allowed in output
   if (!InferFeatures) {
-    for (auto &Feature : Used) {
+    for (auto &Feature : Used.keys()) {
       if (!Out.TargetFeaturesSec->Features.count(Feature))
-        error(Twine("Target feature '") + Feature + "' is not allowed.");
+        error(Twine("Target feature '") + Feature + "' used by " +
+              Used[Feature] + " is not allowed.");
     }
   }
 
   // Validate the required and disallowed constraints for each file
   for (ObjFile *File : Symtab->ObjectFiles) {
+    StringRef FileName(File->getName());
     SmallSet<std::string, 8> ObjectFeatures;
     for (auto &Feature : File->getWasmObj()->getTargetFeatures()) {
       if (Feature.Prefix == WASM_FEATURE_PREFIX_DISALLOWED)
         continue;
       ObjectFeatures.insert(Feature.Name);
       if (Disallowed.count(Feature.Name))
-        error(Twine("Target feature '") + Feature.Name +
-              "' is disallowed. Use --no-check-features to suppress.");
+        error(Twine("Target feature '") + Feature.Name + "' used in " +
+              FileName + " is disallowed by " + Disallowed[Feature.Name] +
+              ". Use --no-check-features to suppress.");
     }
-    for (auto &Feature : Required) {
+    for (auto &Feature : Required.keys()) {
       if (!ObjectFeatures.count(Feature))
-        error(Twine("Missing required target feature '") + Feature +
-              "'. Use --no-check-features to suppress.");
+        error(Twine("Missing target feature '") + Feature + "' in " + FileName +
+              ", required by " + Required[Feature] +
+              ". Use --no-check-features to suppress.");
     }
   }
 }
 
 void Writer::calculateImports() {
   for (Symbol *Sym : Symtab->getSymbols()) {
     if (!Sym->isUndefined())
       continue;