Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: EXLR flagged as error by plugin #121

Merged
Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
4546f99
Commit
SWETAS04 Mar 24, 2021
bd15a10
Sync with development
SWETAS04 Apr 15, 2021
6d141b8
Draft
SWETAS04 Apr 15, 2021
825794b
Instructions affected by this bug added
SWETAS04 Apr 15, 2021
e7e122f
fix
SWETAS04 Apr 19, 2021
c30e506
Merge branch 'development' into Fix_format_to_support_relocatable_sym…
SWETAS04 Apr 19, 2021
b13fd23
Cleanup
SWETAS04 Apr 20, 2021
d58536d
format
SWETAS04 Apr 20, 2021
4c073c0
Sonar scan
SWETAS04 Apr 20, 2021
7c7cb03
..
SWETAS04 Apr 20, 2021
d8b7931
Cleanup
SWETAS04 Apr 20, 2021
d3b6f26
Requested Changes
SWETAS04 May 13, 2021
2ae7f02
Test failing
SWETAS04 May 13, 2021
8c7aa06
cleanup
SWETAS04 May 13, 2021
630575c
Update parser_library/src/diagnostic.cpp
SWETAS04 May 14, 2021
9792622
Merge branch 'development' into Fix_format_to_support_relocatable_sym…
SWETAS04 May 18, 2021
b14b8e2
mnemonics not working fix
SWETAS04 May 18, 2021
88bc54d
Merge branch 'Fix_format_to_support_relocatable_symbols' of https://g…
SWETAS04 May 18, 2021
28aef6b
format
SWETAS04 May 18, 2021
7877561
Conflicts causing build failure
SWETAS04 May 18, 2021
330d4e5
Cleanup
SWETAS04 May 18, 2021
39cfb16
length of 2 bytes error check
SWETAS04 May 19, 2021
cdfbf7e
format
SWETAS04 May 19, 2021
1a7c3c2
Exception for undefined symbol
SWETAS04 May 19, 2021
8563685
...
SWETAS04 May 20, 2021
c47064f
Failing test due to alignement
SWETAS04 May 20, 2021
1585fff
Update diagnostic.h
SWETAS04 May 20, 2021
f0caaca
Update diagnostic.h
SWETAS04 May 20, 2021
f1cbd38
code smell and missing test
SWETAS04 May 24, 2021
3999b01
Position for replaced index not found correctly fix
SWETAS04 May 24, 2021
f1d4749
Delete o
SWETAS04 May 25, 2021
0042876
code smell
SWETAS04 May 25, 2021
b034d5b
Merge branch 'Fix_format_to_support_relocatable_symbols' of https://g…
SWETAS04 May 25, 2021
f5ad18e
...
SWETAS04 May 25, 2021
f978b6c
Merge branch 'development' of https://github.com/eclipse/che-che4z-ls…
SWETAS04 May 27, 2021
5be1b3b
Fix for substitution operands and missing test
SWETAS04 Jun 2, 2021
2f36a6d
Update low_language_processor.cpp
SWETAS04 Jun 2, 2021
0f56a6f
Cleanup
SWETAS04 Jun 2, 2021
92410cf
Macro operands not transformed for RI operands
SWETAS04 Jun 8, 2021
4c80e8f
..
SWETAS04 Jun 8, 2021
75c2054
Merge branch 'development' of https://github.com/eclipse/che-che4z-ls…
SWETAS04 Jun 9, 2021
36c86b3
Test for RI operands called through macro
SWETAS04 Jun 9, 2021
0e30cb7
Update parser_library/src/diagnostic.cpp
SWETAS04 Jun 9, 2021
980bcc5
Requested Changes
SWETAS04 Jun 9, 2021
d8a18f2
Merge branch 'Fix_format_to_support_relocatable_symbols' of https://g…
SWETAS04 Jun 9, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions parser_library/src/checking/instr_operand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,10 @@ hlasm_plugin::parser_library::diagnostic_op machine_operand::get_simple_operand_
return diagnostic_op::error_M111(instr_name, operand_range);
case machine_operand_type::IMM: // I
return diagnostic_op::error_M112(instr_name, operand_range);
case machine_operand_type::REG_IMM: // RI
return diagnostic_op::error_M113(instr_name, operand_range);
case machine_operand_type::VEC_REG: // V
return diagnostic_op::error_M114(instr_name, operand_range);
case machine_operand_type::RELOC_IMM: // RI
return diagnostic_op::error_M113(instr_name, operand_range);
}
assert(false);
return diagnostic_op::error_I999(instr_name, stmt_range);
Expand Down Expand Up @@ -265,7 +265,6 @@ one_operand::one_operand(const one_operand& op)
value = op.value;
is_default = op.is_default;
};

bool one_operand::check(
diagnostic_op& diag, const machine_operand_format to_check, const std::string& instr_name, const range&) const
{
Expand All @@ -287,7 +286,6 @@ bool one_operand::check(
}
return true;
}

// it is a simple operand
if (to_check.identifier.is_signed && !is_size_corresponding_signed(value, to_check.identifier.size))
{
Expand All @@ -297,7 +295,7 @@ bool one_operand::check(
case machine_operand_type::IMM:
diag = diagnostic_op::error_M122(instr_name, -boundary, boundary - 1, operand_range);
break;
case machine_operand_type::REG_IMM:
case machine_operand_type::RELOC_IMM:
michalbali256 marked this conversation as resolved.
Show resolved Hide resolved
diag = diagnostic_op::error_M123(instr_name, -boundary, boundary - 1, operand_range);
break;
default:
Expand Down Expand Up @@ -350,10 +348,6 @@ std::string parameter::to_string() const
return "M";
case machine_operand_type::REG:
return "R";
case machine_operand_type::REG_IMM: {
ret_val = "RI";
break;
}
case machine_operand_type::IMM: {
ret_val = "I";
break;
Expand All @@ -370,6 +364,10 @@ std::string parameter::to_string() const
ret_val = "L";
break;
}
case machine_operand_type::RELOC_IMM: {
ret_val = "RI";
break;
}
case machine_operand_type::VEC_REG:
return "V";
case machine_operand_type::DIS_REG:
Expand Down
4 changes: 2 additions & 2 deletions parser_library/src/checking/instr_operand.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,14 @@ enum class machine_operand_type : uint8_t
{
MASK,
REG,
REG_IMM,
IMM,
NONE,
DISPLC,
BASE,
LENGTH,
VEC_REG,
DIS_REG
DIS_REG,
RELOC_IMM
};

// Describes a component of machine operand format. Specifies allowed values.
Expand Down
90 changes: 45 additions & 45 deletions parser_library/src/context/instruction.cpp

Large diffs are not rendered by default.

21 changes: 13 additions & 8 deletions parser_library/src/context/instruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ const checking::parameter imm_24s = { true, 24, checking::machine_operand_type::
const checking::parameter imm_32s = { true, 32, checking::machine_operand_type::IMM };
const checking::parameter imm_32u = { false, 32, checking::machine_operand_type::IMM };
const checking::parameter vec_reg = { false, 4, checking::machine_operand_type::VEC_REG };
const checking::parameter reg_imm_12s = { true, 12, checking::machine_operand_type::REG_IMM };
const checking::parameter reg_imm_16s = { true, 16, checking::machine_operand_type::REG_IMM };
const checking::parameter reg_imm_24s = { true, 24, checking::machine_operand_type::REG_IMM };
const checking::parameter reg_imm_32s = { true, 32, checking::machine_operand_type::REG_IMM };
const checking::parameter reladdr_imm_12s = { true, 12, checking::machine_operand_type::RELOC_IMM };
const checking::parameter reladdr_imm_16s = { true, 16, checking::machine_operand_type::RELOC_IMM };
const checking::parameter reladdr_imm_24s = { true, 24, checking::machine_operand_type::RELOC_IMM };
const checking::parameter reladdr_imm_32s = { true, 32, checking::machine_operand_type::RELOC_IMM };

/*
Rules for displacement operands:
Expand Down Expand Up @@ -169,10 +169,15 @@ const checking::machine_operand_format imm_32_U = checking::machine_operand_form
const checking::machine_operand_format vec_reg_4_U = checking::machine_operand_format(vec_reg, empty, empty);
const checking::machine_operand_format db_12_8x4L_U = checking::machine_operand_format(dis_12u, length_8, base_);
const checking::machine_operand_format db_12_4x4L_U = checking::machine_operand_format(dis_12u, length_4, base_);
const checking::machine_operand_format reg_imm_12_S = checking::machine_operand_format(reg_imm_12s, empty, empty);
const checking::machine_operand_format reg_imm_16_S = checking::machine_operand_format(reg_imm_16s, empty, empty);
const checking::machine_operand_format reg_imm_24_S = checking::machine_operand_format(reg_imm_24s, empty, empty);
const checking::machine_operand_format reg_imm_32_S = checking::machine_operand_format(reg_imm_32s, empty, empty);
const checking::machine_operand_format rel_addr_imm_12_S =
checking::machine_operand_format(reladdr_imm_12s, empty, empty);
const checking::machine_operand_format rel_addr_imm_16_S =
checking::machine_operand_format(reladdr_imm_16s, empty, empty);
const checking::machine_operand_format rel_addr_imm_24_S =
checking::machine_operand_format(reladdr_imm_24s, empty, empty);
const checking::machine_operand_format rel_addr_imm_32_S =
checking::machine_operand_format(reladdr_imm_32s, empty, empty);


// machine instruction representation for checking
class machine_instruction
Expand Down
18 changes: 16 additions & 2 deletions parser_library/src/diagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,7 @@ diagnostic_op diagnostic_op::error_M113(const std::string& instr_name, const ran
{
return diagnostic_op(diagnostic_severity::error,
"M113",
"Error at " + instr_name + " instruction: operand must be an absolute register immediate value",
"Error at " + instr_name + " instruction: operand must be relocatable symbol or an absolute immediate value",
range);
}

Expand Down Expand Up @@ -1272,7 +1272,7 @@ diagnostic_op diagnostic_op::error_M123(const std::string& instr_name, long long
{
return diagnostic_op(diagnostic_severity::error,
"M123",
"Error at " + instr_name + " instruction: register immediate operand absolute value must be between "
"Error at " + instr_name + " instruction: relocatable symbol or immediate absolute value must be between "
+ std::to_string(from) + " and " + std::to_string(to),
range);
}
Expand Down Expand Up @@ -1477,6 +1477,13 @@ diagnostic_op diagnostic_op::warn_D025(const range& range, const std::string& ty
return diagnostic_op(
diagnostic_severity::warning, "D025", "The " + modifier + " modifier is ignored with type " + type, range);
}
diagnostic_op diagnostic_op::warn_D031(const range& range, const std::string& operand_value)
{
return diagnostic_op(diagnostic_severity::warning,
"D031",
"Operand value " + operand_value + " should be a relocatable symbol",
SWETAS04 marked this conversation as resolved.
Show resolved Hide resolved
range);
}

diagnostic_op diagnostic_op::error_D026(const range& range)
{
Expand Down Expand Up @@ -1857,6 +1864,13 @@ diagnostic_op diagnostic_op::error_ME002(const range& range)
return diagnostic_op(diagnostic_severity::error, "ME002", "multiplication or division of address", range);
}

diagnostic_op diagnostic_op::error_ME003(const range& range)
{
return diagnostic_op(diagnostic_severity::error,
"ME003",
"Relative Immediate Operand value must be signed number of halfwords.",
Copy link
Contributor

Choose a reason for hiding this comment

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

This message implies to me, that the user should write a number as the argument.

Suggested change
"Relative Immediate Operand value must be signed number of halfwords.",
"Relative immediate operand must evaluate into a number of halfwords (must result in even number of bytes)",

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just Relative immediate operand must evaluate into an even offset?

range);
}
diagnostic_op diagnostic_op::error_CE001(const range& range)
{
return diagnostic_op(diagnostic_severity::error, "CE001", "Operator expected", range);
Expand Down
3 changes: 3 additions & 0 deletions parser_library/src/diagnostic.h
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ struct diagnostic_op
static diagnostic_op error_D028(const range& range);
static diagnostic_op error_D029(const range& range);
static diagnostic_op error_D030(const range& range, const std::string& type);
static diagnostic_op warn_D031(const range& range, const std::string& modifier);

static diagnostic_op error_M102(const std::string& instr_name, const range& range);

Expand Down Expand Up @@ -591,6 +592,8 @@ struct diagnostic_op

static diagnostic_op error_ME002(const range& range);

static diagnostic_op error_ME003(const range& range);

static diagnostic_op error_CE001(const range& range);

static diagnostic_op error_CE002(const std::string& message, const range& range);
Expand Down
27 changes: 25 additions & 2 deletions parser_library/src/expressions/mach_operator.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,12 @@ struct sub
static std::string sign_char_begin() { return "-"; }
static std::string sign_char_end() { return ""; }
};

struct rel_addr
{
static std::string sign_char() { return "-"; }
static std::string sign_char_begin() { return "-"; }
static std::string sign_char_end() { return ""; }
};
struct mul
{
static std::string sign_char() { return "*"; }
Expand Down Expand Up @@ -137,7 +142,21 @@ inline mach_expression::value_t mach_expr_binary<sub>::evaluate(mach_evaluate_in
{
return left_->evaluate(info) - right_->evaluate(info);
}
template<>
inline mach_expression::value_t mach_expr_binary<rel_addr>::evaluate(mach_evaluate_info info) const
{
auto location = left_->evaluate(info);
auto target = right_->evaluate(info);
if (target.value_kind() == context::symbol_value_kind::ABS)
{
add_diagnostic(diagnostic_op::warn_D031(get_range(), std::to_string(target.get_abs())));
return target;
}

if ((target - location).value_kind() == context::symbol_value_kind::ABS && (target - location).get_abs() % 2 != 0)
add_diagnostic(diagnostic_op::error_ME003(get_range()));
return target - location;
}
template<>
inline mach_expression::value_t mach_expr_binary<mul>::evaluate(mach_evaluate_info info) const
{
Expand Down Expand Up @@ -198,7 +217,11 @@ inline context::dependency_collector mach_expr_binary<sub>::get_dependencies(mac
{
return left_->get_dependencies(info) - right_->get_dependencies(info);
}

template<>
inline context::dependency_collector mach_expr_binary<rel_addr>::get_dependencies(mach_evaluate_info info) const
{
return left_->get_dependencies(info) - right_->get_dependencies(info);
}
template<>
inline context::dependency_collector mach_expr_binary<mul>::get_dependencies(mach_evaluate_info info) const
{
Expand Down
40 changes: 40 additions & 0 deletions parser_library/src/parsing/parser_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ statement_fields_parser::parse_result parser_impl::parse_operand_field(std::stri
break;
case processing::processing_form::MACH:
line = std::move(h.parser->op_rem_body_mach_r()->line);
transform_imm_reg_operands(line.operands, opcode.value);
break;
case processing::processing_form::DAT:
line = std::move(h.parser->op_rem_body_dat_r()->line);
Expand Down Expand Up @@ -533,6 +534,44 @@ void parser_impl::process_lookahead()
parse_lookahead_operands(std::move(*look_lab_instr->op_text), look_lab_instr->op_range);
}
}
void parser_impl::transform_imm_reg_operands(semantics::operand_list& op_list, id_index instruction)
{
if (instruction->empty())
Copy link
Contributor

Choose a reason for hiding this comment

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

when does this happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a test "parse_bad_model" where it occurs

return;
const machine_instruction* instr;
std::vector<std::pair<size_t, size_t>> replaced;
if (auto mnem_tmp = context::instruction::mnemonic_codes.find(*instruction);
mnem_tmp != context::instruction::mnemonic_codes.end())
{
const auto& mnemonic = context::instruction::mnemonic_codes.at(*instruction);
instr = mnemonic.instruction;
replaced = mnemonic.replaced;
}
else
{
instr = &context::instruction::machine_instructions.at(*instruction);
}
int position = 0;
for (const auto& operand : op_list)
{
for (const auto& [index, value] : replaced)
{
if (index == position)
{
position++;
}
}
if (auto type = instr->operands[position].identifier.type; type == checking::machine_operand_type::RELOC_IMM
&& operand.get()->access_mach() != nullptr && operand.get()->access_mach()->kind == mach_kind::EXPR)
{
auto range = operand.get()->access_mach()->access_expr()->expression.get()->get_range();
mach_expr_ptr& transformed_exp = operand.get()->access_mach()->access_expr()->expression;
transformed_exp = std::make_unique<mach_expr_binary<rel_addr>>(
std::make_unique<mach_expr_location_counter>(range), std::move(transformed_exp), range);
}
position++;
}
}

void parser_impl::parse_operands(const std::string& text, range text_range)
{
Expand Down Expand Up @@ -592,6 +631,7 @@ void parser_impl::parse_operands(const std::string& text, range text_range)
break;
case processing::processing_form::MACH:
h.parser->op_rem_body_mach();
transform_imm_reg_operands(h.parser->collector.current_operands().value, opcode.value);
break;
case processing::processing_form::DAT:
h.parser->op_rem_body_dat();
Expand Down
2 changes: 1 addition & 1 deletion parser_library/src/parsing/parser_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class parser_impl : public antlr4::Parser,

void parse_operands(const std::string& text, range text_range);
void parse_lookahead_operands(const std::string& text, range text_range);

static void transform_imm_reg_operands(semantics::operand_list& op_list, context::id_index instruction);
antlr4::misc::IntervalSet getExpectedTokens() override;

bool input_tokens_invalidated = false;
Expand Down
4 changes: 3 additions & 1 deletion parser_library/src/semantics/collector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ const instruction_si& collector::current_instruction() { return *instr_; }

bool collector::has_instruction() const { return instr_.has_value(); }

const operands_si& collector::current_operands() { return *op_; }

const operands_si& collector::current_operands() const { return *op_; }
operands_si& collector::current_operands() { return *op_; }

const remarks_si& collector::current_remarks() { return *rem_; }

Expand Down
3 changes: 2 additions & 1 deletion parser_library/src/semantics/collector.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ class collector
bool has_label() const;
const instruction_si& current_instruction();
bool has_instruction() const;
const operands_si& current_operands();
const operands_si& current_operands() const;
operands_si& current_operands();
const remarks_si& current_remarks();

void set_label_field(range symbol_range);
Expand Down
37 changes: 23 additions & 14 deletions parser_library/src/semantics/operand_impls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,8 @@ address_machine_operand* machine_operand::access_address()
return kind == mach_kind::ADDR ? static_cast<address_machine_operand*>(this) : nullptr;
}

std::unique_ptr<checking::operand> make_check_operand(expressions::mach_evaluate_info info,
const expressions::mach_expression& expr,
std::optional<checking::machine_operand_type> type_hint = std::nullopt)
std::unique_ptr<checking::operand> make_check_operand(
expressions::mach_evaluate_info info, const expressions::mach_expression& expr)
{
auto res = expr.evaluate(info);
if (res.value_kind() == context::symbol_value_kind::ABS)
Expand All @@ -92,18 +91,24 @@ std::unique_ptr<checking::operand> make_check_operand(expressions::mach_evaluate
}
else
{
if (type_hint && *type_hint == checking::machine_operand_type::REG_IMM)
{
return std::make_unique<checking::one_operand>(0);
}
else
{
return std::make_unique<checking::address_operand>(
checking::address_state::UNRES, 0, 0, 0, checking::operand_state::ONE_OP);
}
return std::make_unique<checking::address_operand>(
checking::address_state::UNRES, 0, 0, 0, checking::operand_state::ONE_OP);
}
}
std::unique_ptr<checking::operand> check_operand(
Copy link
Contributor

Choose a reason for hiding this comment

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

could you rename this method? Now, it is not clear what is the difference between methods check_operand and make_check_operand

expressions::mach_evaluate_info info, const expressions::mach_expression& expr)
{
auto res = expr.evaluate(info);
if (res.value_kind() == context::symbol_value_kind::ABS)
{
return std::make_unique<checking::one_operand>(std::to_string(res.get_abs()), res.get_abs());
}
else
{
return std::make_unique<checking::address_operand>(
checking::address_state::UNRES, 0, 0, 0, checking::operand_state::ONE_OP);
}
}

//***************** expr_machine_operand *********************

expr_machine_operand::expr_machine_operand(expressions::mach_expr_ptr expression, range operand_range)
Expand All @@ -120,7 +125,11 @@ std::unique_ptr<checking::operand> expr_machine_operand::get_operand_value(expre
std::unique_ptr<checking::operand> expr_machine_operand::get_operand_value(
expressions::mach_evaluate_info info, checking::machine_operand_type type_hint) const
{
return make_check_operand(info, *expression, type_hint);
if (type_hint == checking::machine_operand_type::RELOC_IMM)
{
return check_operand(info, *expression);
}
return make_check_operand(info, *expression);
}

// suppress MSVC warning 'inherits via dominance'
Expand Down
Loading