Re: [PATCH v7.1] Support software single step on ARM in GDBServer.

Yao Qi
2015-12-11T14:43:35Z

Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

> + /* Assume all atomic sequences start with a ldrex{,b,h,d} instruction. */
> + insn1 = self->ops->read_memory_unsigned_integer (loc, 2, byte_order_for_code);
> +
> + loc += 2;
> + if (thumb_insn_size (insn1) != 4)
> + return NULL;
> +
> + insn2 = self->ops->read_memory_unsigned_integer (loc, 2, byte_order_for_code);
> +

This line is too long, you may define a macro to shorten
"self->ops->read_memory_unsigned_integer".

> + loc += 2;
> + if (!((insn1 & 0xfff0) == 0xe850
> + || ((insn1 & 0xfff0) == 0xe8d0 && (insn2 & 0x00c0) == 0x0040)))
> + return NULL;
> +
> + /* Assume that no atomic sequence is longer than "atomic_sequence_length"
> + instructions. */
> + for (insn_count = 0; insn_count < atomic_sequence_length; ++insn_count)
> + {
> + insn1
> + = self->ops->read_memory_unsigned_integer (loc, 2,byte_order_for_code);
> + loc += 2;
> +
> + if (thumb_insn_size (insn1) != 4)
> + {
> + /* Assume that there is at most one conditional branch in the
> + atomic sequence. If a conditional branch is found, put a
> + breakpoint in its destination address. */
> + if ((insn1 & 0xf000) == 0xd000 && bits (insn1, 8, 11) != 0x0f)
> + {
> + if (last_breakpoint > 0)
> + return NULL; /* More than one conditional branch found,
> + fallback to the standard code. */
> +
> + breaks[1] = loc + 2 + (sbits (insn1, 0, 7) << 1);
> + last_breakpoint++;
> + }
> +
> + /* We do not support atomic sequences that use any *other*
> + instructions but conditional branches to change the PC.
> + Fall back to standard code to avoid losing control of
> + execution. */
> + else if (thumb_instruction_changes_pc (insn1))
> + return NULL;
> + }
> + else
> + {
> + insn2 = self->ops->read_memory_unsigned_integer
> + (loc, 2, byte_order_for_code);

Format looks wrong, multiple instances of this problem in the patch.

--
Yao (齐尧)