[Bug-Candidate]: slither-read-storage: storage array slots miscalculated in slot layout for shadowed vars

[plotchy]

Describe the issue:

Weird edge case, though this happens with uint256[50] __gap quite often.

If there are instances of shadowed vars, slither-read-storage fails to differentiate them in the storage layout.
Only the first instance of the var is printed, and it uses the last instance's slot for the length holder.

Code example to reproduce the issue:

pragma solidity ^0.8.4;

contract BaseContract{
    uint256 one;
    uint256[50] private __gap;
    uint256 two;
}

contract DerivedContract is BaseContract{
    uint256 three;
    uint256[50] private __gap;
    uint256 four;
}

Version:

0.8.3

Relevant log output:

INFO:Slither-read-storage:
Contract 'DerivedContract'
BaseContract.one with type uint256 is located at slot: 0

INFO:Slither-read-storage:
Name: one
Type: uint256
Slot: 0

INFO:Slither-read-storage:
Contract 'DerivedContract'
DerivedContract.__gap with type uint256[50] is located at slot: 53 <-- ** Should be 1 **

INFO:Slither-read-storage:
Name: __gap
Type: uint256[50]
Slot: 53

INFO:Slither-read-storage:
Contract 'DerivedContract'
DerivedContract.__gap with type uint256[50] is located at slot: 53

INFO:Slither-read-storage:
Key: 0
Name: __gap
Type: uint256
Slot: 53

--- snip ---

INFO:Slither-read-storage:
Contract 'DerivedContract'
DerivedContract.__gap with type uint256[50] is located at slot: 53

INFO:Slither-read-storage:
Key: 19
Name: __gap
Type: uint256
Slot: 72

INFO:Slither-read-storage:
Contract 'DerivedContract'
BaseContract.two with type uint256 is located at slot: 51

INFO:Slither-read-storage:
Name: two
Type: uint256
Slot: 51

INFO:Slither-read-storage:
Contract 'DerivedContract'
DerivedContract.three with type uint256 is located at slot: 52

INFO:Slither-read-storage:
Name: three
Type: uint256
Slot: 52

** DerivedContract.__gap should be here with slot 53 ** 

INFO:Slither-read-storage:
Contract 'DerivedContract'
DerivedContract.four with type uint256 is located at slot: 103

INFO:Slither-read-storage:
Name: four
Type: uint256
Slot: 103

[0xalpharush]

Good find. We should be using contract.state_variables_ordered here:

slither/slither/tools/read_storage/read_storage.py

Line 238 in db703d8

for var in contract.variables

Need to document the difference here:

slither/slither/core/declarations/contract.py

Lines 291 to 311 in db703d8

	def variables(self) -> List["StateVariable"]:
	"""
	list(StateVariable): List of the state variables. Alias to self.state_variables
	"""
	return list(self.state_variables)
	@property
	def variables_as_dict(self) -> Dict[str, "StateVariable"]:
	return self._variables
	@property
	def state_variables(self) -> List["StateVariable"]:
	"""
	list(StateVariable): List of the state variables.
	"""
	return list(self._variables.values())
	@property
	def state_variables_ordered(self) -> List["StateVariable"]:
	"""
	list(StateVariable): List of the state variables by order of declaration.