Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

Output original opcode instead of PUSHC/JUMPC/JUMPCI in VM trace #5852

Merged
merged 2 commits into from
Nov 27, 2019

Conversation

gumb0
Copy link
Member

@gumb0 gumb0 commented Nov 27, 2019

Fixes #5847

cc @holiman

@@ -18,7 +18,7 @@
- Fixed: [#5821](https://github.com/ethereum/aleth/pull/5821) `test_setChainParams` correctly initializes custom configuration of precompiled contracts.
- Fixed: [#5826](https://github.com/ethereum/aleth/pull/5826) Fix blocking bug in database rebuild functionality - users can now rebuild their databases via Aleth's '-R' switch.
- Fixed: [#5827](https://github.com/ethereum/aleth/pull/5827) Detect database upgrades and automatically rebuild the database when they occur.
- Fixed: [#5834](https://github.com/ethereum/aleth/pull/5834) Fix segmentation fault during sync.
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed this entry, as it's already mentioned in 1.7.2

@codecov-io
Copy link

codecov-io commented Nov 27, 2019

Codecov Report

Merging #5852 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5852      +/-   ##
==========================================
+ Coverage   64.06%   64.08%   +0.01%     
==========================================
  Files         362      362              
  Lines       30898    30900       +2     
  Branches     3432     3432              
==========================================
+ Hits        19796    19802       +6     
+ Misses       9874     9871       -3     
+ Partials     1228     1227       -1

@gumb0 gumb0 requested review from halfalicious and chfast November 27, 2019 14:56
Copy link
Member

@chfast chfast left a comment

Choose a reason for hiding this comment

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

Does this resolve to no-op in case there is no tracing callback?

@@ -1582,7 +1583,7 @@ void LegacyVM::interpretCases()
CASE(JUMPCI)
{
#if EVM_REPLACE_CONST_JUMP
ON_OP();
onOperation(Instruction::JUMPC);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
onOperation(Instruction::JUMPC);
onOperation(Instruction::JUMPI);

@gumb0 gumb0 force-pushed the trace-original-push-jump branch from 3d88a34 to 514539a Compare November 27, 2019 15:24
@gumb0
Copy link
Member Author

gumb0 commented Nov 27, 2019

Does this resolve to no-op in case there is no tracing callback?

@chfast Yes, in case callback is not set opOperation does nothing https://github.com/ethereum/aleth/pull/5852/files#diff-214bff4e48034ccf16329f486716aefbR63-R66

There's other definition for ON_OP when EVM_TRACE compile-time flag is set, outputting to stderr instead. These changes don't affect that, but I suspect it's not useful anymore.

#if EVM_TRACE > 0
#undef ON_OP
#if EVM_TRACE > 2
#define ON_OP() \
(cerr << "### " << ++m_nSteps << ": " << m_PC << " " << instructionInfo(m_OP).name << endl)

@gumb0 gumb0 merged commit d3553a5 into master Nov 27, 2019
@gumb0 gumb0 deleted the trace-original-push-jump branch November 27, 2019 16:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LegacyVM with optimizations reports incorrect opcodes in trace
3 participants