Skip to content

Commit

Permalink
fix: Incorrect evaluation of the T attribute in EQU statement
Browse files Browse the repository at this point in the history
  • Loading branch information
slavek-kucera authored Oct 13, 2022
1 parent fd9d73a commit 72dad69
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 50 deletions.
1 change: 1 addition & 0 deletions clients/vscode-hlasmplugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
- Library diagnostics point to a respective configuration file
- Invalid substring may be generated when conditional assembly string contains multibyte UTF-8 characters
- Utilize alignment information during dependency evaluation
- Incorrect evaluation of the T attribute in EQU statement

## [1.4.0](https://github.com/eclipse/che-che4z-lsp-for-hlasm/compare/1.3.0...1.4.0) (2022-08-30)

Expand Down
2 changes: 2 additions & 0 deletions parser_library/src/context/ordinary_assembly/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ target_sources(parser_library PRIVATE
dependant.h
dependency_collector.cpp
dependency_collector.h
dependency_solver_redirect.cpp
dependency_solver_redirect.h
location_counter.cpp
location_counter.h
location_counter_data.cpp
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright (c) 2022 Broadcom.
* The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Broadcom, Inc. - initial API and implementation
*/

#include "dependency_solver_redirect.h"

#include "context/using.h"

namespace hlasm_plugin::parser_library::context {

const symbol* dependency_solver_redirect::get_symbol(id_index name) const { return m_base->get_symbol(name); }

std::optional<address> dependency_solver_redirect::get_loctr() const { return m_base->get_loctr(); }

id_index dependency_solver_redirect::get_literal_id(const std::shared_ptr<const expressions::data_definition>& lit)
{
return m_base->get_literal_id(lit);
}

bool dependency_solver_redirect::using_active(id_index label, const section* sect) const
{
return m_base->using_active(label, sect);
}

using_evaluate_result dependency_solver_redirect::using_evaluate(
id_index label, const section* owner, int32_t offset, bool long_offset) const
{
return m_base->using_evaluate(label, owner, offset, long_offset);
}

std::variant<const symbol*, symbol_candidate> dependency_solver_redirect::get_symbol_candidate(id_index name) const
{
return m_base->get_symbol_candidate(name);
}

std::string dependency_solver_redirect::get_opcode_attr(id_index symbol) const
{
return m_base->get_opcode_attr(symbol);
}

} // namespace hlasm_plugin::parser_library::context
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright (c) 2022 Broadcom.
* The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Broadcom, Inc. - initial API and implementation
*/

#ifndef HLASMPLUGIN_PARSERLIBRARY_CONTEXT_DEPENDENCY_SOLVER_REDIRECT_H
#define HLASMPLUGIN_PARSERLIBRARY_CONTEXT_DEPENDENCY_SOLVER_REDIRECT_H

#include "dependable.h"

namespace hlasm_plugin::parser_library::context {

class dependency_solver_redirect : public dependency_solver
{
dependency_solver* m_base;

public:
const symbol* get_symbol(id_index name) const override;
std::optional<address> get_loctr() const override;
id_index get_literal_id(const std::shared_ptr<const expressions::data_definition>& lit) override;
bool using_active(id_index label, const section* sect) const override;
using_evaluate_result using_evaluate(
id_index label, const section* owner, int32_t offset, bool long_offset) const override;
std::variant<const symbol*, symbol_candidate> get_symbol_candidate(id_index name) const override;
std::string get_opcode_attr(id_index symbol) const override;

protected:
explicit dependency_solver_redirect(dependency_solver& base)
: m_base(&base)
{}

~dependency_solver_redirect() = default;
};

} // namespace hlasm_plugin::parser_library::context

#endif
27 changes: 23 additions & 4 deletions parser_library/src/processing/instruction_sets/asm_processor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,23 @@ void asm_processor::process_LOCTR(rebuilt_statement stmt)
lib_info);
}

struct override_symbol_candidates final : public context::dependency_solver_redirect
{
std::variant<const context::symbol*, context::symbol_candidate> get_symbol_candidate(
context::id_index name) const override
{
if (auto r = dependency_solver_redirect::get_symbol_candidate(name);
std::holds_alternative<const context::symbol*>(r) && std::get<const context::symbol*>(r) == nullptr)
return context::symbol_candidate { false };
else
return r;
}

explicit override_symbol_candidates(context::dependency_solver& solver)
: dependency_solver_redirect(solver)
{}
};

void asm_processor::process_EQU(rebuilt_statement stmt)
{
auto loctr = hlasm_ctx.ord_ctx.align(context::no_align, lib_info);
Expand Down Expand Up @@ -159,9 +176,10 @@ void asm_processor::process_EQU(rebuilt_statement stmt)
auto asm_op = ops[2]->access_asm();
auto expr_op = asm_op->access_expr();

if (expr_op && !expr_op->has_dependencies(dep_solver, nullptr))
override_symbol_candidates dep_solver_override(dep_solver);
if (expr_op && !expr_op->has_dependencies(dep_solver_override, nullptr))
{
auto t_value = expr_op->expression->evaluate(dep_solver, *this);
auto t_value = expr_op->expression->evaluate(dep_solver_override, *this);
if (t_value.value_kind() == context::symbol_value_kind::ABS && t_value.get_abs() >= 0
&& t_value.get_abs() <= 255)
t_attr = (context::symbol_attributes::type_attr)t_value.get_abs();
Expand All @@ -179,9 +197,10 @@ void asm_processor::process_EQU(rebuilt_statement stmt)
auto asm_op = ops[1]->access_asm();
auto expr_op = asm_op->access_expr();

if (expr_op && !expr_op->has_dependencies(dep_solver, nullptr))
override_symbol_candidates dep_solver_override(dep_solver);
if (expr_op && !expr_op->has_dependencies(dep_solver_override, nullptr))
{
auto length_value = expr_op->expression->evaluate(dep_solver, *this);
auto length_value = expr_op->expression->evaluate(dep_solver_override, *this);
if (length_value.value_kind() == context::symbol_value_kind::ABS && length_value.get_abs() >= 0
&& length_value.get_abs() <= 65535)
length_attr = (context::symbol_attributes::len_attr)length_value.get_abs();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,47 +104,14 @@ template class data_def_postponed_statement<checking::data_instr_type::DS>;
template class data_def_dependency<checking::data_instr_type::DC>;
template class data_def_dependency<checking::data_instr_type::DS>;

const context::symbol* data_def_dependency_solver::get_symbol(context::id_index name) const
{
return base.get_symbol(name);
}

std::optional<context::address> data_def_dependency_solver::get_loctr() const
{
if (loctr)
return *loctr + (int)(operands_bit_length / 8);
if (auto l = base.get_loctr(); l.has_value())
if (auto l = dependency_solver_redirect::get_loctr(); l.has_value())
return l.value() + (int)(operands_bit_length / 8);

return std::nullopt;
}

context::id_index data_def_dependency_solver::get_literal_id(
const std::shared_ptr<const expressions::data_definition>& dd)
{
return base.get_literal_id(dd);
}

bool data_def_dependency_solver::using_active(context::id_index label, const context::section* sect) const
{
return base.using_active(label, sect);
}

context::using_evaluate_result data_def_dependency_solver::using_evaluate(
context::id_index label, const context::section* owner, int32_t offset, bool long_offset) const
{
return base.using_evaluate(label, owner, offset, long_offset);
}

std::variant<const context::symbol*, context::symbol_candidate> data_def_dependency_solver::get_symbol_candidate(
context::id_index name) const
{
return base.get_symbol_candidate(name);
}

std::string data_def_dependency_solver::get_opcode_attr(context::id_index name) const
{
return base.get_opcode_attr(name);
}

} // namespace hlasm_plugin::parser_library::processing
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "checking/data_definition/data_def_type_base.h"
#include "context/ordinary_assembly/dependable.h"
#include "context/ordinary_assembly/dependency_solver_redirect.h"
#include "postponed_statement_impl.h"

namespace hlasm_plugin::parser_library::processing {
Expand Down Expand Up @@ -60,26 +61,16 @@ class data_def_postponed_statement final : public postponed_statement_impl
const std::vector<data_def_dependency<instr_type>>& get_dependencies() const { return m_dependencies; }
};

struct data_def_dependency_solver final : public context::dependency_solver
struct data_def_dependency_solver final : public context::dependency_solver_redirect
{
data_def_dependency_solver(context::dependency_solver& base, const context::address* loctr)
: base(base)
: dependency_solver_redirect(base)
, loctr(loctr)
{}

context::dependency_solver& base;
const context::address* loctr;
uint64_t operands_bit_length = 0;

const context::symbol* get_symbol(context::id_index name) const override;
std::optional<context::address> get_loctr() const override;
context::id_index get_literal_id(const std::shared_ptr<const expressions::data_definition>& dd) override;
bool using_active(context::id_index label, const context::section* sect) const override;
context::using_evaluate_result using_evaluate(
context::id_index label, const context::section* owner, int32_t offset, bool long_offset) const override;
std::variant<const context::symbol*, context::symbol_candidate> get_symbol_candidate(
context::id_index name) const override;
std::string get_opcode_attr(context::id_index name) const override;
};

} // namespace hlasm_plugin::parser_library::processing
Expand Down
18 changes: 18 additions & 0 deletions parser_library/test/processing/equ_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,3 +254,21 @@ A EQU ,0

EXPECT_TRUE(matches_message_codes(a.diags(), { "A132" }));
}

TEST(EQU, t_attr_non_existing_symbol)
{
std::string input = R"(
A DS A
B EQU A,*-A,T'TYPO
)";

analyzer a(input);
a.analyze();
a.collect_diags();

EXPECT_TRUE(a.diags().empty());
auto b = get_symbol(a.hlasm_ctx(), "B");
ASSERT_TRUE(b);

EXPECT_EQ(b->attributes().type(), ebcdic_encoding::a2e['U']);
}

0 comments on commit 72dad69

Please sign in to comment.