[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

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

Why not StringRef?

[tlively]

• Use StringRefs

[tlively]

lld/wasm/Writer.cpp

379

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

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/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/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/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/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;