forked from pz9115/corev-binutils-gdb
-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Zcmpe:add cm.push/cm.pop/cm.popret/cm.popretz fail testcase #2
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
歪瑞good |
linsinan1995
pushed a commit
that referenced
this pull request
Mar 30, 2022
linsinan1995
pushed a commit
that referenced
this pull request
Nov 20, 2022
Fedora Rawhide is now using gcc-12.0. As part of updating to the gcc-12.0 package set, Rawhide is also now using a version of libgcc_s which lacks a .data section. This causes gdb to fail in the following fashion while debugging a program (such as gdb) which uses libgcc_s: (top-gdb) run Starting program: rawhide-master/bld/gdb/gdb ... objfiles.h:467: internal-error: sect_index_data not initialized A problem internal to GDB has been detected, further debugging may prove unreliable. ... I snipped the backtrace from the above output. Instead, here's a portion of a backtrace obtained using GDB's backtrace command. (Obviously, in order to obtain it, I used a GDB which has been patched with this commit.) #0 internal_error ( file=0xc6a508 "gdb/objfiles.h", line=467, fmt=0xc6a4e8 "sect_index_data not initialized") at gdbsupport/errors.cc:51 #1 0x00000000005f9651 in objfile::data_section_offset (this=0x4fa48f0) at gdb/objfiles.h:467 #2 0x000000000097c5f8 in relocate_address (address=0x17244, objfile=0x4fa48f0) at gdb/stap-probe.c:1333 #3 0x000000000097c630 in stap_probe::get_relocated_address (this=0xa1a17a0, objfile=0x4fa48f0) at gdb/stap-probe.c:1341 openhwgroup#4 0x00000000004d7025 in create_exception_master_breakpoint_probe ( objfile=0x4fa48f0) at gdb/breakpoint.c:3505 openhwgroup#5 0x00000000004d7426 in create_exception_master_breakpoint () at gdb/breakpoint.c:3575 openhwgroup#6 0x00000000004efcc1 in breakpoint_re_set () at gdb/breakpoint.c:13407 openhwgroup#7 0x0000000000956998 in solib_add (pattern=0x0, from_tty=0, readsyms=1) at gdb/solib.c:1001 openhwgroup#8 0x00000000009576a8 in handle_solib_event () at gdb/solib.c:1269 ... The function 'relocate_address' in gdb/stap-probe.c attempts to do its "relocation" by using objfile->data_section_offset(). That method, data_section_offset() is defined as follows in objfiles.h: CORE_ADDR data_section_offset () const { return section_offsets[SECT_OFF_DATA (this)]; } The internal error occurs when the SECT_OFF_DATA macro finds that the 'sect_index_data' field is -1: #define SECT_OFF_DATA(objfile) \ ((objfile->sect_index_data == -1) \ ? (internal_error (__FILE__, __LINE__, \ _("sect_index_data not initialized")), -1) \ : objfile->sect_index_data) relocate_address() is obtaining the section offset in order to compute a relocated address. For some ABIs, such as the System V ABI, the section offsets will all be the same. So for those ABIs, it doesn't matter which offset is used. However, other ABIs, such as the FDPIC ABI, will have different offsets for the various sections. Thus, for those ABIs, it is vital that this and other relocation code use the correct offset. In stap_probe::get_relocated_address, the address to which to add the offset (thus forming the relocated address) is obtained via this->get_address (); get_address is a getter for m_address in probe.h. It's documented/defined as follows (also in probe.h): /* The address where the probe is inserted, relative to SECT_OFF_TEXT. */ CORE_ADDR m_address; (Thanks to Tom Tromey for this observation.) So, based on this, the current use of data_section_offset / SECT_OFF_DATA is wrong. This relocation code should have been using text_section_offset / SECT_OFF_TEXT all along. That being the case, I've adjusted the stap-probe.c relocation code accordingly. Searching the sources turned up one other use of data_section_offset, in gdb/dtrace-probe.c, so I've updated that code as well. The same reasoning presented above applies to this case too. Summary: * gdb/dtrace-probe.c (dtrace_probe::get_relocated_address): Use method text_section_offset instead of data_section_offset. * gdb/stap-probe.c (relocate_address): Likewise.
linsinan1995
pushed a commit
that referenced
this pull request
Nov 20, 2022
g++ 11.1.0 has a bug where it will emit a negative DW_AT_data_member_location in some cases: $ cat test.cpp #include <memory> int main() { std::unique_ptr<int> ptr; } $ g++ -g test.cpp $ llvm-dwarfdump -F a.out ... 0x00000964: DW_TAG_member DW_AT_name [DW_FORM_strp] ("_M_head_impl") DW_AT_decl_file [DW_FORM_data1] ("/usr/include/c++/11.1.0/tuple") DW_AT_decl_line [DW_FORM_data1] (125) DW_AT_decl_column [DW_FORM_data1] (0x27) DW_AT_type [DW_FORM_ref4] (0x0000067a "default_delete<int>") DW_AT_data_member_location [DW_FORM_sdata] (-1) ... This leads to a GDB crash (when built with ASan, otherwise probably garbage results), since it tries to read just before (to the left, in ASan speak) of the value's buffer: ==888645==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020000c52af at pc 0x7f711b239f4b bp 0x7fff356bd470 sp 0x7fff356bcc18 READ of size 1 at 0x6020000c52af thread T0 #0 0x7f711b239f4a in __interceptor_memcpy /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 #1 0x555c4977efa1 in value_contents_copy_raw /home/simark/src/binutils-gdb/gdb/value.c:1347 #2 0x555c497909cd in value_primitive_field(value*, long, int, type*) /home/simark/src/binutils-gdb/gdb/value.c:3126 #3 0x555c478f2eaa in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:333 openhwgroup#4 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513 openhwgroup#5 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161 openhwgroup#6 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513 openhwgroup#7 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161 openhwgroup#8 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513 openhwgroup#9 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161 openhwgroup#10 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383 openhwgroup#11 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438 openhwgroup#12 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632 openhwgroup#13 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048 openhwgroup#14 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151 openhwgroup#15 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335 openhwgroup#16 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513 openhwgroup#17 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161 openhwgroup#18 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383 openhwgroup#19 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438 openhwgroup#20 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632 openhwgroup#21 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048 openhwgroup#22 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151 openhwgroup#23 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335 openhwgroup#24 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383 openhwgroup#25 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438 openhwgroup#26 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632 openhwgroup#27 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048 openhwgroup#28 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151 openhwgroup#29 0x555c4760f04c in c_value_print(value*, ui_file*, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:587 openhwgroup#30 0x555c483ff954 in language_defn::value_print(value*, ui_file*, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:614 openhwgroup#31 0x555c49759f61 in value_print(value*, ui_file*, value_print_options const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1189 openhwgroup#32 0x555c48950f70 in print_formatted /home/simark/src/binutils-gdb/gdb/printcmd.c:337 openhwgroup#33 0x555c48958eda in print_value(value*, value_print_options const&) /home/simark/src/binutils-gdb/gdb/printcmd.c:1258 openhwgroup#34 0x555c48959891 in print_command_1 /home/simark/src/binutils-gdb/gdb/printcmd.c:1367 openhwgroup#35 0x555c4895a3df in print_command /home/simark/src/binutils-gdb/gdb/printcmd.c:1458 openhwgroup#36 0x555c4767f974 in do_simple_func /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:97 openhwgroup#37 0x555c47692e25 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:2475 openhwgroup#38 0x555c4936107e in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:670 openhwgroup#39 0x555c485f1bff in catch_command_errors /home/simark/src/binutils-gdb/gdb/main.c:523 openhwgroup#40 0x555c485f249c in execute_cmdargs /home/simark/src/binutils-gdb/gdb/main.c:618 openhwgroup#41 0x555c485f6677 in captured_main_1 /home/simark/src/binutils-gdb/gdb/main.c:1317 openhwgroup#42 0x555c485f6c83 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1338 openhwgroup#43 0x555c485f6d65 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1363 openhwgroup#44 0x555c46e41ba8 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32 openhwgroup#45 0x7f71198bcb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24) openhwgroup#46 0x555c46e4197d in _start (/home/simark/build/binutils-gdb-one-target/gdb/gdb+0x77f197d) 0x6020000c52af is located 1 bytes to the left of 8-byte region [0x6020000c52b0,0x6020000c52b8) allocated by thread T0 here: #0 0x7f711b2b7459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154 #1 0x555c470acdc9 in xcalloc /home/simark/src/binutils-gdb/gdb/alloc.c:100 #2 0x555c49b775cd in xzalloc(unsigned long) /home/simark/src/binutils-gdb/gdbsupport/common-utils.cc:29 #3 0x555c4977bdeb in allocate_value_contents /home/simark/src/binutils-gdb/gdb/value.c:1029 openhwgroup#4 0x555c4977be25 in allocate_value(type*) /home/simark/src/binutils-gdb/gdb/value.c:1040 openhwgroup#5 0x555c4979030d in value_primitive_field(value*, long, int, type*) /home/simark/src/binutils-gdb/gdb/value.c:3092 openhwgroup#6 0x555c478f6280 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:501 openhwgroup#7 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161 openhwgroup#8 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513 openhwgroup#9 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161 openhwgroup#10 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513 openhwgroup#11 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161 openhwgroup#12 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383 openhwgroup#13 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438 openhwgroup#14 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632 openhwgroup#15 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048 openhwgroup#16 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151 openhwgroup#17 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335 openhwgroup#18 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513 openhwgroup#19 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161 openhwgroup#20 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383 openhwgroup#21 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438 openhwgroup#22 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632 openhwgroup#23 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048 openhwgroup#24 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151 openhwgroup#25 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335 openhwgroup#26 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383 openhwgroup#27 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438 openhwgroup#28 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632 openhwgroup#29 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048 Since there are some binaries with this in the wild, I think it would be useful for GDB to work around this. I did the obvious simple thing, if the DW_AT_data_member_location's value is -1, replace it with 0. I added a producer check to only apply this fixup for GCC 11. The idea is that if some other compiler ever uses a DW_AT_data_member_location value of -1 by mistake, we don't know (before analyzing the bug at least) if they did mean 0 or some other value. So I wouldn't want to apply the fixup in that case. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28063 Change-Id: Ieef3459b0b9bbce8bdad838ba83b4b64e7269d42
linsinan1995
pushed a commit
that referenced
this pull request
Nov 20, 2022
Starting with commit commit 1da5d0e Date: Tue Jan 4 08:02:24 2022 -0700 Change how Python architecture and language are handled we see a failure in gdb.threads/killed-outside.exp: ... Executing on target: kill -9 16622 (timeout = 300) builtin_spawn -ignore SIGHUP kill -9 16622 continue Continuing. Couldn't get registers: No such process. (gdb) [Thread 0x7ffff77c2700 (LWP 16626) exited] Program terminated with signal SIGKILL, Killed. The program no longer exists. FAIL: gdb.threads/killed-outside.exp: prompt after first continue (timeout) This is not a regression but a failure due to a change in GDB's output. Prior to the aforementioned commit, GDB has been printing the "Couldn't get registers: No such process." message twice. The second one came from (top-gdb) bt #0 amd64_linux_nat_target::fetch_registers (this=0x555557f31440 <the_amd64_linux_nat_target>, regcache=0x555558805ce0, regnum=16) at /gdb-up/gdb/amd64-linux-nat.c:225 #1 0x000055555640ac5f in target_ops::fetch_registers (this=0x555557d636d0 <the_thread_db_target>, arg0=0x555558805ce0, arg1=16) at /gdb-up/gdb/target-delegates.c:502 #2 0x000055555641a647 in target_fetch_registers (regcache=0x555558805ce0, regno=16) at /gdb-up/gdb/target.c:3945 #3 0x0000555556278e68 in regcache::raw_update (this=0x555558805ce0, regnum=16) at /gdb-up/gdb/regcache.c:587 openhwgroup#4 0x0000555556278f14 in readable_regcache::raw_read (this=0x555558805ce0, regnum=16, buf=0x555558881950 "") at /gdb-up/gdb/regcache.c:601 openhwgroup#5 0x00005555562792aa in readable_regcache::cooked_read (this=0x555558805ce0, regnum=16, buf=0x555558881950 "") at /gdb-up/gdb/regcache.c:690 openhwgroup#6 0x000055555627965e in readable_regcache::cooked_read_value (this=0x555558805ce0, regnum=16) at /gdb-up/gdb/regcache.c:748 openhwgroup#7 0x0000555556352a37 in sentinel_frame_prev_register (this_frame=0x555558181090, this_prologue_cache=0x5555581810a8, regnum=16) at /gdb-up/gdb/sentinel-frame.c:53 openhwgroup#8 0x0000555555fa4773 in frame_unwind_register_value (next_frame=0x555558181090, regnum=16) at /gdb-up/gdb/frame.c:1235 openhwgroup#9 0x0000555555fa420d in frame_register_unwind (next_frame=0x555558181090, regnum=16, optimizedp=0x7fffffffd570, unavailablep=0x7fffffffd574, lvalp=0x7fffffffd57c, addrp=0x7fffffffd580, realnump=0x7fffffffd578, bufferp=0x7fffffffd5b0 "") at /gdb-up/gdb/frame.c:1143 openhwgroup#10 0x0000555555fa455f in frame_unwind_register (next_frame=0x555558181090, regnum=16, buf=0x7fffffffd5b0 "") at /gdb-up/gdb/frame.c:1199 openhwgroup#11 0x00005555560178e2 in i386_unwind_pc (gdbarch=0x5555587c4a70, next_frame=0x555558181090) at /gdb-up/gdb/i386-tdep.c:1972 openhwgroup#12 0x0000555555cd2b9d in gdbarch_unwind_pc (gdbarch=0x5555587c4a70, next_frame=0x555558181090) at /gdb-up/gdb/gdbarch.c:3007 openhwgroup#13 0x0000555555fa3a5b in frame_unwind_pc (this_frame=0x555558181090) at /gdb-up/gdb/frame.c:948 openhwgroup#14 0x0000555555fa7621 in get_frame_pc (frame=0x555558181160) at /gdb-up/gdb/frame.c:2572 openhwgroup#15 0x0000555555fa7706 in get_frame_address_in_block (this_frame=0x555558181160) at /gdb-up/gdb/frame.c:2602 openhwgroup#16 0x0000555555fa77d0 in get_frame_address_in_block_if_available (this_frame=0x555558181160, pc=0x7fffffffd708) at /gdb-up/gdb/frame.c:2665 openhwgroup#17 0x0000555555fa5f8d in select_frame (fi=0x555558181160) at /gdb-up/gdb/frame.c:1890 openhwgroup#18 0x0000555555fa5bab in lookup_selected_frame (a_frame_id=..., frame_level=-1) at /gdb-up/gdb/frame.c:1720 openhwgroup#19 0x0000555555fa5e47 in get_selected_frame (message=0x0) at /gdb-up/gdb/frame.c:1810 openhwgroup#20 0x0000555555cc9c6e in get_current_arch () at /gdb-up/gdb/arch-utils.c:848 openhwgroup#21 0x000055555625b239 in gdbpy_before_prompt_hook (extlang=0x555557451f20 <extension_language_python>, current_gdb_prompt=0x555557f4d890 <top_prompt+16> "(gdb) ") at /gdb-up/gdb/python/python.c:1063 openhwgroup#22 0x0000555555f7cfbb in ext_lang_before_prompt (current_gdb_prompt=0x555557f4d890 <top_prompt+16> "(gdb) ") at /gdb-up/gdb/extension.c:922 openhwgroup#23 0x0000555555f7d442 in std::_Function_handler<void (char const*), void (*)(char const*)>::_M_invoke(std::_Any_data const&, char const*&&) (__functor=..., __args#0=@0x7fffffffd900: 0x555557f4d890 <top_prompt+16> "(gdb) ") at /usr/include/c++/7/bits/std_function.h:316 openhwgroup#24 0x0000555555f752dd in std::function<void (char const*)>::operator()(char const*) const (this=0x55555817d838, __args#0=0x555557f4d890 <top_prompt+16> "(gdb) ") at /usr/include/c++/7/bits/std_function.h:706 openhwgroup#25 0x0000555555f75100 in gdb::observers::observable<char const*>::notify (this=0x555557f49060 <gdb::observers::before_prompt>, args#0=0x555557f4d890 <top_prompt+16> "(gdb) ") at /gdb-up/gdb/../gdbsupport/observable.h:150 openhwgroup#26 0x0000555555f736dc in top_level_prompt () at /gdb-up/gdb/event-top.c:444 openhwgroup#27 0x0000555555f735ba in display_gdb_prompt (new_prompt=0x0) at /gdb-up/gdb/event-top.c:411 openhwgroup#28 0x00005555564611a7 in tui_on_command_error () at /gdb-up/gdb/tui/tui-interp.c:205 openhwgroup#29 0x0000555555c2173f in std::_Function_handler<void (), void (*)()>::_M_invoke(std::_Any_data const&) (__functor=...) at /usr/include/c++/7/bits/std_function.h:316 openhwgroup#30 0x0000555555e10c20 in std::function<void ()>::operator()() const (this=0x5555580f9028) at /usr/include/c++/7/bits/std_function.h:706 openhwgroup#31 0x0000555555e10973 in gdb::observers::observable<>::notify() const (this=0x555557f48d20 <gdb::observers::command_error>) at /gdb-up/gdb/../gdbsupport/observable.h:150 openhwgroup#32 0x00005555560e9b3f in start_event_loop () at /gdb-up/gdb/main.c:438 openhwgroup#33 0x00005555560e9bcc in captured_command_loop () at /gdb-up/gdb/main.c:481 openhwgroup#34 0x00005555560eb616 in captured_main (data=0x7fffffffddd0) at /gdb-up/gdb/main.c:1348 openhwgroup#35 0x00005555560eb67c in gdb_main (args=0x7fffffffddd0) at /gdb-up/gdb/main.c:1363 openhwgroup#36 0x0000555555c1b6b3 in main (argc=12, argv=0x7fffffffded8) at /gdb-up/gdb/gdb.c:32 Commit 1da5d0e eliminated the call to 'get_current_arch' in 'gdbpy_before_prompt_hook'. Hence, the second instance of "Couldn't get registers: No such process." does not appear anymore. Fix the failure by updating the regular expression in the test.
linsinan1995
pushed a commit
that referenced
this pull request
Nov 20, 2022
…ync." Commit 14b3360 ("do_target_wait_1: Clear TARGET_WNOHANG if the target isn't async.") broke some multi-target tests, such as gdb.multi/multi-target-info-inferiors.exp. The symptom is that execution just hangs at some point. What happens is: 1. One remote inferior is started, and now sits stopped at a breakpoint. It is not "async" at this point (but it "can async"). 2. We run a native inferior, the event loop gets woken up by the native target's fd. 3. In do_target_wait, we randomly choose an inferior to call target_wait on first, it happens to be the remote inferior. 4. Because the target is currently not "async", we clear TARGET_WNOHANG, resulting in synchronous wait. We therefore block here: #0 0x00007fe9540dbb4d in select () from /usr/lib/libc.so.6 #1 0x000055fc7e821da7 in gdb_select (n=15, readfds=0x7ffdb77c1fb0, writefds=0x0, exceptfds=0x7ffdb77c2050, timeout=0x7ffdb77c1f90) at /home/simark/src/binutils-gdb/gdb/posix-hdep.c:31 #2 0x000055fc7ddef905 in interruptible_select (n=15, readfds=0x7ffdb77c1fb0, writefds=0x0, exceptfds=0x7ffdb77c2050, timeout=0x7ffdb77c1f90) at /home/simark/src/binutils-gdb/gdb/event-top.c:1134 #3 0x000055fc7eda58e4 in ser_base_wait_for (scb=0x6250002e4100, timeout=1) at /home/simark/src/binutils-gdb/gdb/ser-base.c:240 openhwgroup#4 0x000055fc7eda66ba in do_ser_base_readchar (scb=0x6250002e4100, timeout=-1) at /home/simark/src/binutils-gdb/gdb/ser-base.c:365 openhwgroup#5 0x000055fc7eda6ff6 in generic_readchar (scb=0x6250002e4100, timeout=-1, do_readchar=0x55fc7eda663c <do_ser_base_readchar(serial*, int)>) at /home/simark/src/binutils-gdb/gdb/ser-base.c:444 openhwgroup#6 0x000055fc7eda718a in ser_base_readchar (scb=0x6250002e4100, timeout=-1) at /home/simark/src/binutils-gdb/gdb/ser-base.c:471 openhwgroup#7 0x000055fc7edb1ecd in serial_readchar (scb=0x6250002e4100, timeout=-1) at /home/simark/src/binutils-gdb/gdb/serial.c:393 openhwgroup#8 0x000055fc7ec48b8f in remote_target::readchar (this=0x617000038780, timeout=-1) at /home/simark/src/binutils-gdb/gdb/remote.c:9446 openhwgroup#9 0x000055fc7ec4da82 in remote_target::getpkt_or_notif_sane_1 (this=0x617000038780, buf=0x6170000387a8, forever=1, expecting_notif=1, is_notif=0x7ffdb77c24f0) at /home/simark/src/binutils-gdb/gdb/remote.c:9928 openhwgroup#10 0x000055fc7ec4f045 in remote_target::getpkt_or_notif_sane (this=0x617000038780, buf=0x6170000387a8, forever=1, is_notif=0x7ffdb77c24f0) at /home/simark/src/binutils-gdb/gdb/remote.c:10037 openhwgroup#11 0x000055fc7ec354d4 in remote_target::wait_ns (this=0x617000038780, ptid=..., status=0x7ffdb77c33c8, options=...) at /home/simark/src/binutils-gdb/gdb/remote.c:8147 openhwgroup#12 0x000055fc7ec38aa1 in remote_target::wait (this=0x617000038780, ptid=..., status=0x7ffdb77c33c8, options=...) at /home/simark/src/binutils-gdb/gdb/remote.c:8337 openhwgroup#13 0x000055fc7f1409ce in target_wait (ptid=..., status=0x7ffdb77c33c8, options=...) at /home/simark/src/binutils-gdb/gdb/target.c:2612 openhwgroup#14 0x000055fc7e19da98 in do_target_wait_1 (inf=0x617000038080, ptid=..., status=0x7ffdb77c33c8, options=...) at /home/simark/src/binutils-gdb/gdb/infrun.c:3636 openhwgroup#15 0x000055fc7e19e26b in operator() (__closure=0x7ffdb77c2f90, inf=0x617000038080) at /home/simark/src/binutils-gdb/gdb/infrun.c:3697 openhwgroup#16 0x000055fc7e19f0c4 in do_target_wait (ecs=0x7ffdb77c33a0, options=...) at /home/simark/src/binutils-gdb/gdb/infrun.c:3716 openhwgroup#17 0x000055fc7e1a31f7 in fetch_inferior_event () at /home/simark/src/binutils-gdb/gdb/infrun.c:4061 Before the aforementioned commit, we would not have cleared TARGET_WNOHANG, the remote target's wait would have returned nothing, and we would have consumed the native target's event. After applying this revert, the testsuite state looks as good as before for me on Ubuntu 20.04 amd64. Change-Id: Ic17a1642935cabcc16c25cb6899d52e12c2f5c3f
linsinan1995
pushed a commit
that referenced
this pull request
Nov 20, 2022
The current zombie leader detection code in linux-nat.c has a race -- if a multi-threaded inferior exits just before check_zombie_leaders finds that the leader is now zombie via checking /proc/PID/status, check_zombie_leaders deletes the leader, assuming we won't get an event for that exit (which we won't in some scenarios, but not in this one). That might seem mostly harmless, but it has some downsides: - later when we continue pulling events out of the kernel, we will collect the exit event of the non-leader threads, and once we see the last lwp in our list exit, we return _that_ lwp's exit code as whole-process exit code to infrun, instead of the leader's exit code. - this can cause a hang in stop_all_threads in infrun.c. Say there are 2 threads in the process. stop_all_threads stops each of those threads, and then waits for two stop or exit events, one for each thread. If the whole process exits, and check_zombie_leaders hits the false-positive case, linux-nat.c will only return one event to GDB (the whole-process exit returned when we see the last thread, the non-leader thread, exit), making stop_all_threads hang forever waiting for a second event that will never come. However, in this false-positive scenario, where the whole process is exiting, as opposed to just the leader (with pthread_exit(), for example), we _will_ get an exit event shortly for the leader, after we collect the exit event of all the other non-leader threads. Or put another way, we _always_ get an event for the leader after we see it become zombie. I tried a number of approaches to fix this: #1 - My first thought to address the race was to make GDB always report the whole-process exit status for the leader thread, not for whatever is the last lwp in the list. We _always_ get a final exit (or exec) event for the leader, and when the race triggers, we're not collecting it. #2 - My second thought was to try to plug the race in the first place. I thought of making GDB call waitpid/WNOHANG for all non-leader threads immediately when the zombie leader is detected, assuming there would be an exit event pending for each of them waiting to be collected. Turns out that that doesn't work -- you can see the leader become zombie _before_ the kernel kills all other threads. Waitpid in that small time window returns 0, indicating no-event. Thankfully we hit that race window all the time, which avoided trading one race for another. Looking at the non-leader thread's status in /proc doesn't help either, the threads are still in running state for a bit, for the same reason. #3 - My next attempt, which seemed promising, was to synchronously stop and wait for the stop for each of the non-leader threads. For the scenario in question, this will collect all the exit statuses of the non-leader threads. Then, if we are left with only the zombie leader in the lwp list, it means we either have a normal while-process exit or an exec, in which case we should not delete the leader. If _only_ the leader exited, like in gdb.threads/leader-exit.exp, then after pausing threads, we will still have at least one live non-leader thread in the list, and so we delete the leader lwp. I got this working and polished, and it was only after staring at the kernel code to convince myself that this would really work (and it would, for the scenario I considered), that I realized I had failed to account for one scenario -- if any non-leader thread is _already_ stopped when some thread triggers a group exit, like e.g., if you have some threads stopped and then resume just one thread with scheduler-locking or non-stop, and that thread exits the process. I also played with PTRACE_EVENT_EXIT, see if it would help in any way to plug the race, and I couldn't find a way that it would result in any practical difference compared to looking at /proc/PID/status, with respect to having a race. So I concluded that there's no way to plug the race, we just have to deal with it. Which means, going back to approach #1. That is the approach taken by this patch. Change-Id: I6309fd4727da8c67951f9cea557724b77e8ee979
linsinan1995
pushed a commit
that referenced
this pull request
Nov 20, 2022
Bug 28980 shows that trying to value_copy an entirely optimized out value causes an internal error. The original bug report involves MI and some Python pretty printer, and is quite difficult to reproduce, but another easy way to reproduce (that is believed to be equivalent) was proposed: $ ./gdb -q -nx --data-directory=data-directory -ex "py print(gdb.Value(gdb.Value(5).type.optimized_out()))" /home/smarchi/src/binutils-gdb/gdb/value.c:1731: internal-error: value_copy: Assertion `arg->contents != nullptr' failed. This is caused by 5f8ab46 ("gdb: constify parameter of value_copy"). It added an assertion that the contents buffer is allocated if the value is not lazy: if (!value_lazy (val)) { gdb_assert (arg->contents != nullptr); This was based on the comment on value::contents, which suggest that this is the case: /* Actual contents of the value. Target byte-order. NULL or not valid if lazy is nonzero. */ gdb::unique_xmalloc_ptr<gdb_byte> contents; However, it turns out that it can also be nullptr also if the value is entirely optimized out, for example on exit of allocate_optimized_out_value. That function creates a lazy value, marks the entire value as optimized out, and then clears the lazy flag. But contents remains nullptr. This wasn't a problem for value_copy before, because it was calling value_contents_all_raw on the input value, which caused contents to be allocated before doing the copy. This means that the input value to value_copy did not have its contents allocated on entry, but had it allocated on exit. The result value had it allocated on exit. And that we copied bytes for an entirely optimized out value (i.e. meaningless bytes). From here I see two choices: 1. respect the documented invariant that contents is nullptr only and only if the value is lazy, which means making allocate_optimized_out_value allocate contents 2. extend the cases where contents can be nullptr to also include values that are entirely optimized out (note that you could still have some entirely optimized out values that do have contents allocated, it depends on how they were created) and adjust value_copy accordingly Choice #1 is safe, but less efficient: it's not very useful to allocate a buffer for an entirely optimized out value. It's even a bit less efficient than what we had initially, because values coming out of allocate_optimized_out_value would now always get their contents allocated. Choice #2 would be more efficient than what we had before: giving an optimized out value without allocated contents to value_copy would result in an optimized out value without allocated contents (and the input value would still be without allocated contents on exit). But it's more risky, since it's difficult to ensure that all users of the contents (through the various_contents* accessors) are all fine with that new invariant. In this patch, I opt for choice #2, since I think it is a better direction than choice #1. #1 would be a pessimization, and if we go this way, I doubt that it will ever be revisited, it will just stay that way forever. Add a selftest to test this. I initially started to write it as a Python test (since the reproducer is in Python), but a selftest is more straightforward. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28980 Change-Id: I6e2f5c0ea804fafa041fcc4345d47064b5900ed7
linsinan1995
pushed a commit
that referenced
this pull request
Nov 20, 2022
While working on a different patch, I triggered an assertion from the initialize_current_architecture code, specifically from one of the *_gdbarch_init functions in a *-tdep.c file. This exposes a couple of issues with GDB. This is easy enough to reproduce by adding 'gdb_assert (false)' into a suitable function. For example, I added a line into i386_gdbarch_init and can see the following issue. I start GDB and immediately hit the assert, the output is as you'd expect, except for the very last line: $ ./gdb/gdb --data-directory ./gdb/data-directory/ ../../src.dev-1/gdb/i386-tdep.c:8455: internal-error: i386_gdbarch_init: Assertion `false' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. ----- Backtrace ----- ... snip ... --------------------- ../../src.dev-1/gdb/i386-tdep.c:8455: internal-error: i386_gdbarch_init: Assertion `false' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Quit this debugging session? (y or n) ../../src.dev-1/gdb/ser-event.c:212:16: runtime error: member access within null pointer of type 'struct serial' Something goes wrong when we try to query the user. Note, I configured GDB with --enable-ubsan, I suspect that without this the above "error" would actually just be a crash. The backtrace from ser-event.c:212 looks like this: (gdb) bt 10 #0 serial_event_clear (event=0x675c020) at ../../src/gdb/ser-event.c:212 #1 0x0000000000769456 in invoke_async_signal_handlers () at ../../src/gdb/async-event.c:211 #2 0x000000000295049b in gdb_do_one_event () at ../../src/gdbsupport/event-loop.cc:194 #3 0x0000000001f015f8 in gdb_readline_wrapper ( prompt=0x67135c0 "../../src/gdb/i386-tdep.c:8455: internal-error: i386_gdbarch_init: Assertion `false' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugg"...) at ../../src/gdb/top.c:1141 openhwgroup#4 0x0000000002118b64 in defaulted_query(const char *, char, typedef __va_list_tag __va_list_tag *) ( ctlstr=0x2e4eb68 "%s\nQuit this debugging session? ", defchar=0 '\000', args=0x7fffffffa6e0) at ../../src/gdb/utils.c:934 openhwgroup#5 0x0000000002118f72 in query (ctlstr=0x2e4eb68 "%s\nQuit this debugging session? ") at ../../src/gdb/utils.c:1026 openhwgroup#6 0x00000000021170f6 in internal_vproblem(internal_problem *, const char *, int, const char *, typedef __va_list_tag __va_list_tag *) (problem=0x6107bc0 <internal_error_problem>, file=0x2b976c8 "../../src/gdb/i386-tdep.c", line=8455, fmt=0x2b96d7f "%s: Assertion `%s' failed.", ap=0x7fffffffa8e8) at ../../src/gdb/utils.c:417 openhwgroup#7 0x00000000021175a0 in internal_verror (file=0x2b976c8 "../../src/gdb/i386-tdep.c", line=8455, fmt=0x2b96d7f "%s: Assertion `%s' failed.", ap=0x7fffffffa8e8) at ../../src/gdb/utils.c:485 openhwgroup#8 0x00000000029503b3 in internal_error (file=0x2b976c8 "../../src/gdb/i386-tdep.c", line=8455, fmt=0x2b96d7f "%s: Assertion `%s' failed.") at ../../src/gdbsupport/errors.cc:55 openhwgroup#9 0x000000000122d5b6 in i386_gdbarch_init (info=..., arches=0x0) at ../../src/gdb/i386-tdep.c:8455 (More stack frames follow...) It turns out that the problem is that the async event handler mechanism has been invoked, but this has not yet been initialized. If we look at gdb_init (in gdb/top.c) we can indeed see the call to gdb_init_signals is after the call to initialize_current_architecture. If I reorder the calls, moving gdb_init_signals earlier, then the initial error is resolved, however, things are still broken. I now see the same "Quit this debugging session? (y or n)" prompt, but when I provide an answer and press return GDB immediately crashes. So what's going on now? The next problem is that the call_readline field within the current_ui structure is not initialized, and this callback is invoked to process the reply I entered. The problem is that call_readline is setup as a result of calling set_top_level_interpreter, which is called from captured_main_1. Unfortunately, set_top_level_interpreter is called after gdb_init is called. I wondered how to solve this problem for a while, however, I don't know if there's an easy "just reorder some lines" solution here. Looking through captured_main_1 there seems to be a bunch of dependencies between printing various things, parsing config files, and setting up the interpreter. I'm sure there is a solution hiding in there somewhere.... I'm just not sure I want to spend any longer looking for it. So. I propose a simpler solution, more of a hack/work-around. In utils.c we already have a function filtered_printing_initialized, this is checked in a few places within internal_vproblem. In some of these cases the call gates whether or not GDB will query the user. My proposal is to add a new readline_initialized function, which checks if the current_ui has had readline initialized yet. If this is not the case then we should not attempt to query the user. After this change GDB prints the error message, the backtrace, and then aborts (including dumping core). This actually seems pretty sane as, if GDB has not yet made it through the initialization then it doesn't make much sense to allow the user to say "no, I don't want to quit the debug session" (I think).
linsinan1995
pushed a commit
that referenced
this pull request
Nov 20, 2022
The variable right_lib_flags is not being set correctly to define RIGHT. The value RIGHT is needed to force the address of the library functions lib1_func3 and lib2_func4 to occur at different address in the wrong and right libraries. With RIGHT defined correctly, functions lib1_func3 and lib2_func4 occur at different addresses the test runs correctly on Powerpc. The test needs the lib2 addresses to be different in the right and wrong cases. That is the point of introducing function lib2_spacer with the ifdef RIGHT compiler directive. On Intel, the ARRAY_SIZE of 1 versus 8192 is sufficient to get the dynamic linker to move the addresses of the library. You can also get the same effect on PowerPC but you must use a value much larger than 8192. The key thing is that the test was not properly setting RIGHT to defined to get the lib2_spacer function on Intel and Powerpc. Without the patch, we have the Intel backtrace for the bad libraries: backtrace #0 break_here () at /home/ ... /gdb/testsuite/gdb.base/solib-search.c:30 #1 0x00007ffff7fae156 in ?? () #2 0x00007fffffffc150 in ?? () #3 0x00007ffff7fbb156 in ?? () openhwgroup#4 0x00007fffffffc160 in ?? () openhwgroup#5 0x00007ffff7fae146 in ?? () openhwgroup#6 0x00007fffffffc170 in ?? () openhwgroup#7 0x00007ffff7fbb146 in ?? () openhwgroup#8 0x00007fffffffc180 in ?? () openhwgroup#9 0x0000555555555156 in main () at /home/ ... /binutils-gdb/gdb/testsuite/gdb.base/solib-search.c:23 Backtrace stopped: previous frame inner to this frame (corrupt stack?) (gdb) PASS: gdb.base/solib-search.exp: backtrace (with wrong libs) (data collection) The backtrace on Intel with the good libraries is: backtrace #0 break_here () at /.../binutils-gdb/gdb/testsuite/gdb.base/solib-search.c:30 #1 0x00007ffff7fae156 in lib2_func4 () at /.../binutils-gdb/gdb/testsuite/gdb.base/solib-search-lib2.c:49 #2 0x00007ffff7fbb156 in lib1_func3 () at /.../gdb.base/solib-search-lib1.c:49 #3 0x00007ffff7fae146 in lib2_func2 () at /.../testsuite/gdb.base/solib-search-lib2.c:30 openhwgroup#4 0x00007ffff7fbb146 in lib1_func1 () at /.../gdb.base/solib-search-lib1.c:30 openhwgroup#5 0x0000555555555156 in main () at /...solib-search.c:23 (gdb) PASS: gdb.base/solib-search.exp: backtrace (with right libs) (data collection) PASS: gdb.base/solib-search.exp: backtrace (with right libs) In one case the backtrace is correct and the other it is wrong on Intel. This is due to the fact that the ARRAY_SIZE caused the dynamic linker to move the library function addresses around. I believe it has to do with the default size of the data and code sections used by the dynamic linker. So without the patch the backtrace on PowerPC looks like: backtrace #0 break_here () at /.../solib-search.c:30 #1 0x00007ffff7f007f4 in lib2_func4 () at /.../solib-search-lib2.c:49 #2 0x00007ffff7f307f4 in lib1_func3 () at /.../solib-search-lib1.c:49 #3 0x00007ffff7f007ac in lib2_func2 () at /.../solib-search-lib2.c:30 openhwgroup#4 0x00007ffff7f307ac in lib1_func1 () at /.../solib-search-lib1.c:30 openhwgroup#5 0x000000001000074c in main () at /.../solib-search.c:23 for both the good and bad libraries. The patch fixes defining RIGHT in solib-search-lib1.c and solib-search- lib2.c. Note, without the patch the lib1_spacer and lib2_spacer functions do not show up in the object dump of the Intel or Powerpc libraries as it should. The patch fixes that by making sure RIGHT gets defined. Now with the patch the backtrace for the bad library on PowerPC looks like: backtrace #0 break_here () at /.../solib-search.c:30 #1 0x00007ffff7f0083c in __glink_PLTresolve () from /.../solib-search-lib2.so Backtrace stopped: frame did not save the PC And the backtrace for the good libraries on PowerPC looks like: backtrace #0 break_here () at /.../solib-search.c:30 #1 0x00007ffff7f0083c in lib2_func4 () at /.../solib-search-lib2.c:49 #2 0x00007ffff7f3083c in lib1_func3 () at /.../solib-search-lib1.c:49 #3 0x00007ffff7f007cc in lib2_func2 () at /.../solib-search-lib2.c:30 openhwgroup#4 0x00007ffff7f307cc in lib1_func1 () at /.../solib-search-lib1.c:30 openhwgroup#5 0x000000001000074c in main () at /.../solib-search.c:23 (gdb) PASS: gdb.base/solib-search.exp: backtrace (with right libs) (data collection) PASS: gdb.base/solib-search.exp: backtrace (with right libs) The issue then is on Power where the ARRAY_SIZE of 1 versus 8192 is not sufficient to cause the dymanic linker to allocate the libraries at different addresses. I don't claim to understand the specifics of how the dynamic linker works and what the default size is for the data and code sections are. My guess is by default PowerPC allocates a larger data size by default, which is large enough to hold array[8192]. The default size of the data section allocated by the dynamic linker on Intel is not large enough to hold array[8192] thus causing the code section on Intel to have to move when the large array is defined. Note on PowerPC, if you make ARRAY_SIZE big enough, then you will cause the library addresses to occur at different addresses as the larger data section forces the code section to a different address. That was actually my original fix for the program until I spoke with Doug Evans who originally wrote the test. Doug noticed that RIGHT was not getting defined as he originally intended in the test. With the patch to fix the definition of RIGHT, PowerPC has a bad and a good backtrace because the address of lib1_func3 and lib2_func4 both move because lib1_spacer and lib2_spacer are now defined before lib1_func3 and lib2_func4. Without the patch, the lib1_spacer and lib2_spacer function doesn't show up in the binary for the correct or incorrect library on Intel or PowerPC. With the patch, RIGHT gets defined as originally intended for the test on both architectures and lib1_spacer and lib2_spacer function show up in the binaries on both architectures changing the other function addresses as intended thus causing the test work as intended on PowerPC.
linsinan1995
pushed a commit
that referenced
this pull request
Nov 20, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
please review it.