Skip to content

Conversation

@parth-07
Copy link
Contributor

This commit fixes the parsing / interpretation of "archive:mem" file pattern. Previously, it was getting incorrectly stored because we were not properly handling quotes when a single quoted token in the linker script ("archive:member") is stored as two separate patterns in the linker implementation.

Resolves #680

@parth-07 parth-07 force-pushed the ArchiveMemPattern branch 2 times, most recently from 11b25c9 to 90f1426 Compare December 29, 2025 20:22
// We cannot store the original quote information because we represent
// the single token "archive:mem" in the linker script as two separate
// patterns in the linker codebase.
Tok = unquote(Tok);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use createParserStr to preserve the property that the input token had a quote.

The map file might not show information about quotes from the linker script or the linker script generator too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createParserStr alone would not be helpful here. We do use createParserStr within createAndRegisterWildcardPattern. The issue is that a single quoted token ("archive:mem") is stored as two separate tokens -- archive file pattern and member pattern -- in the linker codebase. It would be incorrect to store both archive file pattern and the member pattern with quotes if the original combined pattern was with quotes because "archive":"mem" would be interpreted differently from "archive:mem".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we have to seperate the file pattern from the member pattern, but I could not follow why "archive":"mem" needs to be interpreted differently from "archive:mem".

The quotes need to be still printed in the map file to preserve how we read the linker script right ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem in the current function createParserStr is that it assumes that when a string starts with quote, it ends with quote as well. So the pattern split appears to have an issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we have to seperate the file pattern from the member pattern, but I could not follow why "archive":"mem" needs to be interpreted differently from "archive:mem".

The quotes need to be still printed in the map file to preserve how we read the linker script right ?

If the linker script contains "libfoo.a:1.o" and we keep the quotes on both the archive pattern and the member pattern and consequently store them as "libfoo.a" and "1.o", then when we print the rule in the map-file as "libfoo.a":"1.o" because we added the quotes in both the archive pattern and the member pattern. "libfoo.a":"1.o" will be interpreted differently from "libfoo.a:1.o". Thus, we will need extra conditions to properly print the rule if we preserve the quotes in both the archive pattern and the member pattern.

Should I make the change to add quotes on both the archive and the member pattern and add the logic to properly print the rule description when both the archive pattern and the member pattern contain quotes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current code assumes that if the string started with a quote, it will also end up with a quote.

So this pattern "libfoo.a:1.o" will need to be treated as

  • "libfoo.a
  • 1.o"

StrToken will need to know of if the state of the string has a single quote/double quote, just for printing purposes only.

The representation for linker script matching will still be libfoo.a and 1.o.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated StrToken such that it can store the single quote information.

// We cannot store the original quote information because we represent
// the single token "archive:mem" in the linker script as two separate
// patterns in the linker codebase.
Tok = unquote(Tok);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??

This commit fixes the parsing / interpretation of `"archive:mem"` file
pattern. Previously, it was getting incorrectly stored because we were
not properly handling quotes when a single quoted token in the
linker script (`"archive:member"`) is stored as two separate patterns
in the linker implementation.

Resolves qualcomm#680

Signed-off-by: Parth Arora <partaror@qti.qualcomm.com>
Copy link
Contributor

@quic-seaswara quic-seaswara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diff --git a/lib/Script/ScriptFile.cpp b/lib/Script/ScriptFile.cpp
index 8c048bb9..00feb8f9 100644
--- a/lib/Script/ScriptFile.cpp
+++ b/lib/Script/ScriptFile.cpp
@@ -538,13 +538,28 @@ StrToken *ScriptFile::createParserStr(const char *PText, size_t PLength) {

 StrToken *ScriptFile::createParserStr(llvm::StringRef S) {
   bool IsQuoted = false;
+  bool IsLeftQuoted = false;
+  bool IsRightQuoted = false;
   if (S.starts_with("\"")) {
-    S = S.substr(1, S.size() - 2);
-    IsQuoted = true;
+    if (S.ends_with("\""))  {
+      S = S.substr(1, S.size() - 2);
+      IsQuoted = true;
+    }
+    else {
+      S = S.substr(1, S.size() - 1);
+      IsLeftQuoted = true;
+    }
+  } else if (S.ends_with("\"")) {
+    S = S.substr(0, S.size() - 1);
+    IsRightQuoted = true;
   }
   StrToken *Tok = make<eld::StrToken>(S.str());
   if (IsQuoted)
     Tok->setQuoted();
+  if (IsLeftQuoted)
+    Tok->setLeftQuote();
+  if (IsRightQuoted)
+    Tok->setRightQuote();
   return Tok;
 }

diff --git a/lib/ScriptParser/ScriptParser.cpp b/lib/ScriptParser/ScriptParser.cpp
index 4e0b2047..126a8405 100644
--- a/lib/ScriptParser/ScriptParser.cpp
+++ b/lib/ScriptParser/ScriptParser.cpp
@@ -661,18 +661,11 @@ InputSectDesc::Spec ScriptParser::readInputSectionDescSpec(StringRef Tok) {
   if (!Tok.contains(':'))
     FilePat = createAndRegisterWildcardPattern(Tok);
   else {
-    bool isQuoted = Tok.starts_with("\"");
-    Tok = unquote(Tok);
     std::pair<llvm::StringRef, llvm::StringRef> Split = Tok.split(':');
     FilePat = createAndRegisterWildcardPattern(Split.first);
     if (!Split.second.empty()) {
       ArchiveMem = createAndRegisterWildcardPattern(Split.second);
     }
-    if (isQuoted) {
-      FilePat->setLeftQuote();
-      if (ArchiveMem)
-        ArchiveMem->setRightQuote();
-    }
     llvm::StringRef peekTok = peek();
     if (!atEOF() && peekTok != "(" &&
         computeLineNumber(peekTok) == getCurrentLineNumber()) {

I was approaching the problem in the way above.

The core issue is in createParserStr.

Can you please look into it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"archive:mem" input section description is not interpreted correctly when it is within quotes

2 participants