[MC] Support skip and count for .incbin directive

[phosek]

Summary

These optional arguments are supported by GNU assembler.

[davide]

Do you mind to add one or two tests for the error condition unexpected token in ?

[davide]

lib/MC/MCParser/AsmParser.cpp

4407–4411 ↗

(On Diff #71764)

this and the lines at 4413-4417 are identical. Can you factor out in a common helper?

[phosek]

I did some more testing against GNU as and found out that it also allows skip to be empty, this behavior is now supported as well. I've also expanded error checking for negative skip and count values.

[davide]

Cool. LGTM. Do you want me to review also the other two patches you have in queue?

[phosek]

That would be awesome, they're not really related to this one but I'd like to land them as well.

lib/MC/MCParser/AsmParser.cpp

4407–4411 ↗

(On Diff #71764)

This no longer relevant as these blocks are now different.

llvm/trunk/lib/MC/MCParser/AsmParser.cpp

private:
   }
   static void DiagHandler(const SMDiagnostic &Diag, void *Context);
 
   /// \brief Enter the specified file. This returns true on failure.
   bool enterIncludeFile(const std::string &Filename);
 
   /// \brief Process the specified file for the .incbin directive.
   /// This returns true on failure.
-  bool processIncbinFile(const std::string &Filename);
+  bool processIncbinFile(const std::string &Filename, int64_t Skip = 0,
+                         const MCExpr *Count = nullptr, SMLoc Loc = SMLoc());
 
   /// \brief Reset the current lexer position to that given by \p Loc. The
   /// current token is not set; clients should ensure Lex() is called
   /// subsequently.
   ///
   /// \param InBuffer If not 0, should be the known buffer id that contains the
   /// location.
   void jumpToLoc(SMLoc Loc, unsigned InBuffer = 0);
bool AsmParser::enterIncludeFile(const std::string &Filename) {
   CurBuffer = NewBuf;
   Lexer.setBuffer(SrcMgr.getMemoryBuffer(CurBuffer)->getBuffer());
   return false;
 }
 
 /// Process the specified .incbin file by searching for it in the include paths
 /// then just emitting the byte contents of the file to the streamer. This
 /// returns true on failure.
-bool AsmParser::processIncbinFile(const std::string &Filename) {
+bool AsmParser::processIncbinFile(const std::string &Filename, int64_t Skip,
+                                  const MCExpr *Count, SMLoc Loc) {
   std::string IncludedFile;
   unsigned NewBuf =
       SrcMgr.AddIncludeFile(Filename, Lexer.getLoc(), IncludedFile);
   if (!NewBuf)
     return true;
 
   // Pick up the bytes from the file and emit them.
-  getStreamer().EmitBytes(SrcMgr.getMemoryBuffer(NewBuf)->getBuffer());
+  StringRef Bytes = SrcMgr.getMemoryBuffer(NewBuf)->getBuffer();
+  Bytes = Bytes.drop_front(Skip);
+  if (Count) {
+    int64_t Res;
+    if (!Count->evaluateAsAbsolute(Res))
+      return Error(Loc, "expected absolute expression");
+    if (Res < 0)
+      return Warning(Loc, "negative count has no effect");
+    Bytes = Bytes.take_front(Res);
+  }
+  getStreamer().EmitBytes(Bytes);
   return false;
 }
 
 void AsmParser::jumpToLoc(SMLoc Loc, unsigned InBuffer) {
   CurBuffer = InBuffer ? InBuffer : SrcMgr.FindBufferContainingLoc(Loc);
   Lexer.setBuffer(SrcMgr.getMemoryBuffer(CurBuffer)->getBuffer(),
                   Loc.getPointer());
 }
if (check(getTok().isNot(AsmToken::String),
       check(enterIncludeFile(Filename), IncludeLoc,
             "Could not find include file '" + Filename + "'"))
     return true;
 
   return false;
 }
 
 /// parseDirectiveIncbin
-///  ::= .incbin "filename"
+///  ::= .incbin "filename" [ , skip [ , count ] ]
 bool AsmParser::parseDirectiveIncbin() {
   // Allow the strings to have escaped octal character sequence.
   std::string Filename;
   SMLoc IncbinLoc = getTok().getLoc();
   if (check(getTok().isNot(AsmToken::String),
             "expected string in '.incbin' directive") ||
-      parseEscapedString(Filename) ||
-      parseToken(AsmToken::EndOfStatement,
+      parseEscapedString(Filename))
+    return true;
+
+  int64_t Skip = 0;
+  const MCExpr *Count = nullptr;
+  SMLoc SkipLoc, CountLoc;
+  if (getLexer().isNot(AsmToken::EndOfStatement)) {
+    if (parseToken(AsmToken::Comma, "unexpected token in '.incbin' directive"))
+      return true;
+
+    // The skip expression can be omitted while specifying the count, e.g:
+    //  .incbin "filename",,4
+    if (getTok().isNot(AsmToken::Comma)) {
+      if (parseTokenLoc(SkipLoc) || parseAbsoluteExpression(Skip))
+        return true;
+    }
+
+    if (getLexer().isNot(AsmToken::EndOfStatement)) {
+      if (parseToken(AsmToken::Comma, "unexpected token in '.incbin' directive") ||
+          parseTokenLoc(CountLoc) || parseExpression(Count))
+        return true;
+    }
+  }
+
+  if (parseToken(AsmToken::EndOfStatement,
                  "unexpected token in '.incbin' directive"))
     return true;
 
+  if (check(Skip < 0, SkipLoc, "skip is negative"))
+    return true;
+
   // Attempt to process the included file.
-  if (processIncbinFile(Filename))
+  if (processIncbinFile(Filename, Skip, Count, CountLoc))
     return Error(IncbinLoc, "Could not find incbin file '" + Filename + "'");
   return false;
 }
 
 /// parseDirectiveIf
 /// ::= .if{,eq,ge,gt,le,lt,ne} expression
 bool AsmParser::parseDirectiveIf(SMLoc DirectiveLoc, DirectiveKind DirKind) {
   TheCondStack.push_back(TheCondState);

llvm/trunk/test/MC/AsmParser/directive_incbin.s

-# RUN: llvm-mc -triple i386-unknown-unknown %s -I %p | FileCheck %s
+# RUN: not llvm-mc -triple i386-unknown-unknown %s -I %p | FileCheck %s
+# RUN: not llvm-mc -triple i386-unknown-unknown %s -I %p 2>&1 > /dev/null| FileCheck %s --check-prefix=CHECK-ERROR
 
 .data
 .incbin "incbin\137abcd"  # "\137" is underscore "_"
 
 # CHECK: .ascii  "abcd\n"
+
+.data
+.incbin "incbin\137abcd", 1
+
+# CHECK: .ascii  "bcd\n"
+
+.data
+.incbin "incbin\137abcd", 1, 2
+
+# CHECK: .ascii  "bc"
+
+.data
+.incbin "incbin\137abcd",, 2
+
+# CHECK: .ascii  "ab"
+
+.data
+.incbin incbin\137abcd
+
+# CHECK-ERROR: error: expected string in '.incbin' directive
+
+.data
+.incbin "incbin\137abcd" 1
+
+# CHECK-ERROR: error: unexpected token in '.incbin' directive
+
+.data
+.incbin "incbin\137abcd", 1 2
+
+# CHECK-ERROR: error: unexpected token in '.incbin' directive
+
+.data
+.incbin "incbin\137abcd", -1
+
+# CHECK-ERROR: error: skip is negative
+
+.data
+.incbin "incbin\137abcd",, -1
+
+# CHECK-ERROR: warning: negative count has no effect