Skip to content

Commit

Permalink
[AsmParser][SystemZ][z/OS] Re-introduce HLASM comment syntax
Browse files Browse the repository at this point in the history
- https://reviews.llvm.org/rGb605cfb336989705f391d255b7628062d3dfe9c3 was reverted due to sanitizer bugs in the introduced unit-test (specifically in the Address sanitizer https://lab.llvm.org/buildbot/#/builders/5/builds/5697)
- This patch attempts to rectify that, as well as re-factor parts of the test
- The issue was previously, within the `setupCallToAsmParser` function in the unit-test, `SrcMgr` was declared as a local variable. `SrcMgr` owns a unique pointer. Since the variable goes out of scope at the end of the function, the unique pointer is released.
- This patch, moves the declaration of the `SrcMgr` variable to a class field, since the scope will remain until the class's destructor is invoked (which in this case is at the end of the unit test)
- Furthermore, this patch also moves the `MCContext Ctx` declaration from a local variable instance inside a function, to a unique pointer class field. This ensures the instantiation of the MCContext remains until the tear down of the test.

Reviewed By: abhina.sreeskantharajan

Differential Revision: https://reviews.llvm.org/D99004
  • Loading branch information
aniprasad committed Mar 24, 2021
1 parent b3386a7 commit 301d926
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 2 deletions.
9 changes: 8 additions & 1 deletion llvm/include/llvm/MC/MCAsmInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,14 @@ class MCAsmInfo {
/// other when on the same line. Defaults to ';'
const char *SeparatorString;

/// This indicates the comment character used by the assembler. Defaults to
/// This indicates the comment string used by the assembler. Defaults to
/// "#"
StringRef CommentString;

/// This indicates whether the comment string is only accepted as a comment
/// at the beginning of statements. Defaults to false.
bool RestrictCommentStringToStartOfStatement = false;

/// This is appended to emitted labels. Defaults to ":"
const char *LabelSuffix;

Expand Down Expand Up @@ -557,6 +561,9 @@ class MCAsmInfo {
unsigned getCommentColumn() const { return 40; }

StringRef getCommentString() const { return CommentString; }
bool getRestrictCommentStringToStartOfStatement() const {
return RestrictCommentStringToStartOfStatement;
}
const char *getLabelSuffix() const { return LabelSuffix; }

bool useAssignmentForEHBegin() const { return UseAssignmentForEHBegin; }
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/MC/MCParser/AsmLexer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,9 @@ size_t AsmLexer::peekTokens(MutableArrayRef<AsmToken> Buf,
}

bool AsmLexer::isAtStartOfComment(const char *Ptr) {
if (MAI.getRestrictCommentStringToStartOfStatement() && !IsAtStartOfStatement)
return false;

StringRef CommentString = MAI.getCommentString();

if (CommentString.size() == 1)
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ SystemZMCAsmInfo::SystemZMCAsmInfo(const Triple &TT) {

MaxInstLength = 6;

CommentString = "#";
CommentString = AssemblerDialect == AD_HLASM ? "*" : "#";
RestrictCommentStringToStartOfStatement = (AssemblerDialect == AD_HLASM);
ZeroDirective = "\t.space\t";
Data64bitsDirective = "\t.quad\t";
UsesELFSectionDirectiveForBSS = true;
Expand Down
14 changes: 14 additions & 0 deletions llvm/unittests/MC/SystemZ/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
include_directories(
${LLVM_MAIN_SRC_DIR}/lib/Target/SystemZ
)

set(LLVM_LINK_COMPONENTS
SystemZ
MCParser
MC
Support
)

add_llvm_unittest(SystemZAsmLexerTests
SystemZAsmLexerTest.cpp
)
155 changes: 155 additions & 0 deletions llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
//===- llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp ----------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===--------------------------------------------------------------------===//
#include "llvm/MC/MCAsmInfo.h"
#include "llvm/MC/MCContext.h"
#include "llvm/MC/MCObjectFileInfo.h"
#include "llvm/MC/MCParser/MCTargetAsmParser.h"
#include "llvm/MC/MCRegisterInfo.h"
#include "llvm/MC/MCStreamer.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/SourceMgr.h"
#include "llvm/Support/TargetRegistry.h"
#include "llvm/Support/TargetSelect.h"

#include "gtest/gtest.h"

using namespace llvm;

namespace {

// Come up with our hacked version of MCAsmInfo.
// This hacked version derives from the main MCAsmInfo instance.
// Here, we're free to override whatever we want, without polluting
// the main MCAsmInfo interface.
class MockedUpMCAsmInfo : public MCAsmInfo {
public:
void setRestrictCommentStringToStartOfStatement(bool Value) {
RestrictCommentStringToStartOfStatement = Value;
}
void setCommentString(StringRef Value) { CommentString = Value; }
};

// Setup a testing class that the GTest framework can call.
class SystemZAsmLexerTest : public ::testing::Test {
protected:
static void SetUpTestCase() {
LLVMInitializeSystemZTargetInfo();
LLVMInitializeSystemZTargetMC();
}

std::unique_ptr<MCRegisterInfo> MRI;
std::unique_ptr<MockedUpMCAsmInfo> MUPMAI;
std::unique_ptr<const MCInstrInfo> MII;
std::unique_ptr<MCStreamer> Str;
std::unique_ptr<MCAsmParser> Parser;
std::unique_ptr<MCContext> Ctx;

SourceMgr SrcMgr;
std::string TripleName;
llvm::Triple Triple;
const Target *TheTarget;

const MCTargetOptions MCOptions;
MCObjectFileInfo MOFI;

SystemZAsmLexerTest() {
// We will use the SystemZ triple, because of missing
// Object File and Streamer support for the z/OS target.
TripleName = "s390x-ibm-linux";
Triple = llvm::Triple(TripleName);

std::string Error;
TheTarget = TargetRegistry::lookupTarget(TripleName, Error);
EXPECT_NE(TheTarget, nullptr);

MRI.reset(TheTarget->createMCRegInfo(TripleName));
EXPECT_NE(MRI, nullptr);

std::unique_ptr<MCAsmInfo> MAI;
MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
EXPECT_NE(MAI, nullptr);

// Now we cast to our mocked up version of MCAsmInfo.
MUPMAI.reset(static_cast<MockedUpMCAsmInfo *>(MAI.release()));
// MUPMAI should "hold" MAI.
EXPECT_NE(MUPMAI, nullptr);
// After releasing, MAI should now be null.
EXPECT_EQ(MAI, nullptr);
}

void setupCallToAsmParser(StringRef AsmStr) {
std::unique_ptr<MemoryBuffer> Buffer(MemoryBuffer::getMemBuffer(AsmStr));
SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
EXPECT_EQ(Buffer, nullptr);

Ctx.reset(
new MCContext(MUPMAI.get(), MRI.get(), &MOFI, &SrcMgr, &MCOptions));
MOFI.InitMCObjectFileInfo(Triple, false, *Ctx, false);

Str.reset(TheTarget->createNullStreamer(*Ctx));

Parser.reset(createMCAsmParser(SrcMgr, *Ctx, *Str, *MUPMAI));
// Lex initially to get the string.
Parser->getLexer().Lex();
}

void lexAndCheckTokens(StringRef AsmStr,
SmallVector<AsmToken::TokenKind> ExpectedTokens) {
// Get reference to AsmLexer.
MCAsmLexer &Lexer = Parser->getLexer();
// Loop through all expected tokens checking one by one.
for (size_t I = 0; I < ExpectedTokens.size(); ++I) {
EXPECT_EQ(Lexer.getTok().getKind(), ExpectedTokens[I]);
Lexer.Lex();
}
}
};

TEST_F(SystemZAsmLexerTest, CheckDontRestrictCommentStringToStartOfStatement) {
StringRef AsmStr = "jne #-4";

// Setup.
setupCallToAsmParser(AsmStr);

SmallVector<AsmToken::TokenKind> ExpectedTokens(
{AsmToken::Identifier, AsmToken::EndOfStatement});
lexAndCheckTokens(AsmStr /* "jne #-4" */, ExpectedTokens);
}

// Testing MCAsmInfo's RestrictCommentStringToStartOfStatement attribute.
TEST_F(SystemZAsmLexerTest, CheckRestrictCommentStringToStartOfStatement) {
StringRef AsmStr = "jne #-4";

// Setup.
MUPMAI->setRestrictCommentStringToStartOfStatement(true);
setupCallToAsmParser(AsmStr);

// When we are restricting the comment string to only the start of the
// statement, The sequence of tokens we are expecting are: Identifier - "jne"
// Hash - '#'
// Minus - '-'
// Integer - '4'
SmallVector<AsmToken::TokenKind> ExpectedTokens(
{AsmToken::Identifier, AsmToken::Hash, AsmToken::Minus,
AsmToken::Integer});
lexAndCheckTokens(AsmStr /* "jne #-4" */, ExpectedTokens);
}

// Test HLASM Comment Syntax ('*')
TEST_F(SystemZAsmLexerTest, CheckHLASMComment) {
StringRef AsmStr = "* lhi 1,10";

// Setup.
MUPMAI->setCommentString("*");
setupCallToAsmParser(AsmStr);

SmallVector<AsmToken::TokenKind> ExpectedTokens(
{AsmToken::EndOfStatement, AsmToken::Eof});
lexAndCheckTokens(AsmStr /* "* lhi 1,10" */, ExpectedTokens);
}
} // end anonymous namespace

0 comments on commit 301d926

Please sign in to comment.