[Lldb-commits] [lldb] r283847 - Add a first unit test for the arm64 instruction profiled unwind

[Zachary Turner via lldb-commits]

On Mon, Oct 10, 2016 at 7:33 PM Jason Molenda via lldb-commits <
lldb-commits at lists.llvm.org> wrote:

>
> +    if (process_sp->GetTarget().ReadMemory(
>
> +            range.GetBaseAddress(), prefer_file_cache,
> function_text.data(),
>
> +            range.GetByteSize(), error) != range.GetByteSize()) {
>
> +      return false;
>
> +    }
>
> +  }
>
> +  return GetNonCallSiteUnwindPlanFromAssembly(
>
> +      range, function_text.data(), function_text.size(), unwind_plan);
>
> +}
>
> +
>
> +bool UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly(
>
> +    AddressRange &range, uint8_t *opcode_data, size_t opcode_size,
>
> +    UnwindPlan &unwind_plan) {
>
If you pass a MutableArrayRef<uint8_t> the code is a bit safer, since llvm
will assert if you try to index out of bounds, rather than allowing you to
silently access the invalid memory.


>
>
>
>    bool
>
> +  GetNonCallSiteUnwindPlanFromAssembly(lldb_private::AddressRange &func,
>
> +                                       uint8_t *opcode_data, size_t
> opcode_size,
>
> +                                       lldb_private::UnwindPlan
> &unwind_plan);
>
> +
>
> +  bool
>
Same here.


>
>    AugmentUnwindPlanFromCallSite(lldb_private::AddressRange &func,
>
>                                  lldb_private::Thread &thread,
>
>                                  lldb_private::UnwindPlan &unwind_plan)
> override;
>
> @@ -67,8 +72,8 @@ private:
>
>    UnwindAssemblyInstEmulation(const lldb_private::ArchSpec &arch,
>
>                                lldb_private::EmulateInstruction
> *inst_emulator)
>
>        : UnwindAssembly(arch), m_inst_emulator_ap(inst_emulator),
>
> -        m_range_ptr(NULL), m_thread_ptr(NULL), m_unwind_plan_ptr(NULL),
>
> -        m_curr_row(), m_cfa_reg_info(), m_fp_is_cfa(false),
> m_register_values(),
>
> +        m_range_ptr(NULL), m_unwind_plan_ptr(NULL), m_curr_row(),
>
> +        m_cfa_reg_info(), m_fp_is_cfa(false), m_register_values(),
>
>          m_pushed_regs(), m_curr_row_modified(false),
>
>          m_forward_branch_offset(0) {
>
Should we initialize these inline at the point of declaration rather than
in the initializer list?  It makes the constructor code a little less
verbose.


>
> +TEST_F(TestArm64InstEmulation, TestSimpleFunction) {
>
> +
>
> +  init();
>
> +
>
> +  ArchSpec arch("arm64-apple-ios10", nullptr);
>
> +  UnwindAssemblyInstEmulation *engine =
>
> +      static_cast<UnwindAssemblyInstEmulation *>(
>
> +          UnwindAssemblyInstEmulation::CreateInstance(arch));
>
> +  EXPECT_TRUE(engine != nullptr);
>
> +  if (engine == nullptr)
>
> +    return;
>
Use ASSERT_NE(nullptr, engine);  If it is nullptr, the macro will return
automatically for you.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20161011/e3c594f9/attachment.html>