Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1445633002:
[Android] Upstream resource_sizes and check_static_initializers and add chartjson support (Closed)
|
Created:
5 years, 1 month ago by rnephew (Reviews Here) Modified:
4 years, 10 months ago CC:
chromium-reviews, klundberg+watch_chromium.org, mikecase+watch_chromium.org, yfriedman+watch_chromium.org, jbudorick+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Android] Upstream resource_sizes and check_static_initializers and add chartjson support
BUG=546012
Committed: https://crrev.com/f10ebab18d15241a1bd7c0b790a9256e4cafa623
Cr-Commit-Position: refs/heads/master@{#365933}
Patch Set 1 #
Total comments: 2
Patch Set 2 : move typechecks to begining of functions #
Total comments: 10
Patch Set 3 : #Patch Set 4 : #Patch Set 5 : fix imports #
Total comments: 27
Patch Set 6 : #Patch Set 7 : cmd_helper #
Total comments: 2
Patch Set 8 : #
Total comments: 6
Patch Set 9 : #Patch Set 10 : #
Total comments: 2
Messages
Total messages: 36 (7 generated)
Upstreaming of resource_sizes.py Changes for pylint errors and adding support for chartjson format for perf dashboard. I also pulled in the pieces used by clank_check_static_initializers.py I do not see anywhere else that uses clank_check_static_initializers, but I'll look a little more and make sure. It only uses a small portion of that script, so if nowhere else uses it I will kill that script when I kill the downstream version of resource_sizes.py
lgtm https://codereview.chromium.org/1445633002/diff/1/build/android/resource_size... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/1/build/android/resource_size... build/android/resource_sizes.py:170: if not isinstance(chartjson, dict): nit: either remove this assertion or put it at the beginning of the method as: if chartjson and not isinstance...
https://codereview.chromium.org/1445633002/diff/1/build/android/resource_size... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/1/build/android/resource_size... build/android/resource_sizes.py:170: if not isinstance(chartjson, dict): On 2015/11/13 19:54:41, agrieve wrote: > nit: either remove this assertion or put it at the beginning of the method as: > if chartjson and not isinstance... Done.
https://codereview.chromium.org/1445633002/diff/20001/build/android/resource_... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/20001/build/android/resource_... build/android/resource_sizes.py:78: def add_value(chart_data, graph_title, trace_title, value, units, style: it's no longer a private function, so should be AddValue, or maybe AddValueToChartJson. https://codereview.chromium.org/1445633002/diff/20001/build/android/resource_... build/android/resource_sizes.py:172: add_value(chartjson, apk_basename + '_Breakdown', group.name + ' size', There seems to be a lot of repetition everywhere when reporting values, either to chartjson, or to PrintPerfResult. I would suggest to either: - move the "if chartjson" check inside add_value (maybe call it ReportPerfResult instead), which can then either add to the chartjson dict or print the value to report; or - always add to chartjson dict and then, at the end of main, you decide whether to dump the chartjson to a file, or basically translate the set of values in the chartjson to corresponding PrintPerfResult calls. https://codereview.chromium.org/1445633002/diff/20001/build/android/resource_... build/android/resource_sizes.py:372: help='Path to libchrome.so with symbols') nit: end all help sentences with "." https://codereview.chromium.org/1445633002/diff/20001/build/android/resource_... build/android/resource_sizes.py:389: option_parser.error('Must set --output-dir if using chartjson format') maybe just drop to the current directory instead of raising an error? https://codereview.chromium.org/1445633002/diff/20001/build/android/resource_... build/android/resource_sizes.py:393: option_parser.error('Must set --chartjson if you set --output-dir') Maybe it's OK to just ignore the value --output-dir if --chartjson is missing?
https://codereview.chromium.org/1445633002/diff/20001/build/android/resource_... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/20001/build/android/resource_... build/android/resource_sizes.py:78: def add_value(chart_data, graph_title, trace_title, value, units, On 2015/11/16 10:22:34, perezju wrote: > style: it's no longer a private function, so should be AddValue, or maybe > AddValueToChartJson. Done. https://codereview.chromium.org/1445633002/diff/20001/build/android/resource_... build/android/resource_sizes.py:172: add_value(chartjson, apk_basename + '_Breakdown', group.name + ' size', On 2015/11/16 10:22:34, perezju wrote: > There seems to be a lot of repetition everywhere when reporting values, either > to chartjson, or to PrintPerfResult. > > I would suggest to either: > - move the "if chartjson" check inside add_value (maybe call it ReportPerfResult > instead), which can then either add to the chartjson dict or print the value to > report; or > - always add to chartjson dict and then, at the end of main, you decide whether > to dump the chartjson to a file, or basically translate the set of values in the > chartjson to corresponding PrintPerfResult calls. Moved to ReportPerfResults. https://codereview.chromium.org/1445633002/diff/20001/build/android/resource_... build/android/resource_sizes.py:372: help='Path to libchrome.so with symbols') On 2015/11/16 10:22:34, perezju wrote: > nit: end all help sentences with "." Done. https://codereview.chromium.org/1445633002/diff/20001/build/android/resource_... build/android/resource_sizes.py:389: option_parser.error('Must set --output-dir if using chartjson format') On 2015/11/16 10:22:34, perezju wrote: > maybe just drop to the current directory instead of raising an error? Done. https://codereview.chromium.org/1445633002/diff/20001/build/android/resource_... build/android/resource_sizes.py:393: option_parser.error('Must set --chartjson if you set --output-dir') On 2015/11/16 10:22:34, perezju wrote: > Maybe it's OK to just ignore the value --output-dir if --chartjson is missing? Done.
lgtm w/nits also, just to double-check, this already includes the necessary fixes for the numbers that were wrongly computed in the previous version of this script? https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:24: sys.path.append( optional nit: define _CHROMIUM_SRC as os.path.join(os.path.dirname(__file__), os.pardir, os.pardir); and then the other paths relative to _CHROMIUM_SRC https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:110: os.path.getsize(f), 'bytes') nit: indentation is off, should be ReportPerfResult(chartjson, ... os.path.getsize(f), ...) or ReportPerfResult( chartjson, ... os.path.getsize(f), ...) https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:174: apk_size, 'bytes') nit: indentation (everywhere in this function)
https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:24: sys.path.append( On 2015/11/17 09:39:33, perezju wrote: > optional nit: define _CHROMIUM_SRC as os.path.join(os.path.dirname(__file__), > os.pardir, os.pardir); and then the other paths relative to _CHROMIUM_SRC that or pylib.constants.DIR_SOURCE_ROOT https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:70: handle = subprocess.Popen(['nm', so_path], stdout=subprocess.PIPE) subprocess -> cmd_helper https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:81: improvement_direction='down', important=True): nit: indentation https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:94: 'imporvement_direction': improvement_direction, nit: imporvement -> improvement https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:173: ReportPerfResult(chartjson, apk_basename + '_Breakdown', group.name + ' size', nit: line length https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:218: total_compress_size = sum([pak.compress_size for pak in paks]) nit: shouldn't be a list, just sum(pak.compress_size for pak in paks) https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:219: total_file_size = sum([pak.file_size for pak in paks]) nit: same https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:228: for pak in sorted(paks, key=operator.attrgetter('file_size'), reverse=True): did not know about operator.attrgetter. https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:264: reverse=True): nit: indentation https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:275: out_dir = os.path.join(sys.path[0], '..', '..', 'out', build_type) change this https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:285: rc_header_re = re.compile(r'^#define (?P<name>\w+) (?P<id>\d+)$') pull this out to module scope & compile it there.
For the size changes, I thought (based on comment #9 in the bug) that that was already fixed. I commented in the bug to see if that is the case. https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:24: sys.path.append( On 2015/11/17 14:45:21, jbudorick wrote: > On 2015/11/17 09:39:33, perezju wrote: > > optional nit: define _CHROMIUM_SRC as os.path.join(os.path.dirname(__file__), > > os.pardir, os.pardir); and then the other paths relative to _CHROMIUM_SRC > > that or pylib.constants.DIR_SOURCE_ROOT Done. https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:81: improvement_direction='down', important=True): On 2015/11/17 14:45:20, jbudorick wrote: > nit: indentation Done. https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:94: 'imporvement_direction': improvement_direction, On 2015/11/17 14:45:20, jbudorick wrote: > nit: imporvement -> improvement Done. https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:110: os.path.getsize(f), 'bytes') On 2015/11/17 09:39:33, perezju wrote: > nit: indentation is off, should be > > ReportPerfResult(chartjson, ... > os.path.getsize(f), ...) > > or > > ReportPerfResult( > chartjson, ... > os.path.getsize(f), ...) Done. https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:173: ReportPerfResult(chartjson, apk_basename + '_Breakdown', group.name + ' size', On 2015/11/17 14:45:20, jbudorick wrote: > nit: line length These were all a :%s/a/b change that I didn't bother fixing indententations for. All fixed. https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:174: apk_size, 'bytes') On 2015/11/17 09:39:33, perezju wrote: > nit: indentation (everywhere in this function) Done. https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:218: total_compress_size = sum([pak.compress_size for pak in paks]) On 2015/11/17 14:45:21, jbudorick wrote: > nit: shouldn't be a list, just > > sum(pak.compress_size for pak in paks) Done. https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:219: total_file_size = sum([pak.file_size for pak in paks]) On 2015/11/17 14:45:20, jbudorick wrote: > nit: same Done. https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:264: reverse=True): On 2015/11/17 14:45:20, jbudorick wrote: > nit: indentation Done. https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:275: out_dir = os.path.join(sys.path[0], '..', '..', 'out', build_type) On 2015/11/17 14:45:20, jbudorick wrote: > change this Done. https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:285: rc_header_re = re.compile(r'^#define (?P<name>\w+) (?P<id>\d+)$') On 2015/11/17 14:45:20, jbudorick wrote: > pull this out to module scope & compile it there. Done.
https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:70: handle = subprocess.Popen(['nm', so_path], stdout=subprocess.PIPE) On 2015/11/17 14:45:20, jbudorick wrote: > subprocess -> cmd_helper Done.
Andrew, can you confirm whether the numbers provided by the script were already accurate? Randy, could you also double-check and do a simple comparison between the results produced by this script and your apksize.py? https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... build/android/resource_sizes.py:228: for pak in sorted(paks, key=operator.attrgetter('file_size'), reverse=True): On 2015/11/17 14:45:20, jbudorick wrote: > did not know about operator.attrgetter. I usually prefer key=lambda d: d.filse_size, because it's both shorter and more explicit. But I don't have any strong objections against attrgetter either. https://codereview.chromium.org/1445633002/diff/120001/build/android/resource... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/120001/build/android/resource... build/android/resource_sizes.py:71: _, output = cmd_helper.GetCmdStatusAndOutput(['nm', so_path]) if you're going to ignore the exit status use GetCmdOutput
On 2015/11/18 09:55:41, perezju wrote: > Andrew, can you confirm whether the numbers provided by the script were already > accurate? > > Randy, could you also double-check and do a simple comparison between the > results produced by this script and your apksize.py? > > https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... > File build/android/resource_sizes.py (right): > > https://codereview.chromium.org/1445633002/diff/80001/build/android/resource_... > build/android/resource_sizes.py:228: for pak in sorted(paks, > key=operator.attrgetter('file_size'), reverse=True): > On 2015/11/17 14:45:20, jbudorick wrote: > > did not know about operator.attrgetter. > > I usually prefer key=lambda d: d.filse_size, because it's both shorter and more > explicit. But I don't have any strong objections against attrgetter either. > > https://codereview.chromium.org/1445633002/diff/120001/build/android/resource... > File build/android/resource_sizes.py (right): > > https://codereview.chromium.org/1445633002/diff/120001/build/android/resource... > build/android/resource_sizes.py:71: _, output = > cmd_helper.GetCmdStatusAndOutput(['nm', so_path]) > if you're going to ignore the exit status use GetCmdOutput Yep, the numbers are correct afaict.
https://codereview.chromium.org/1445633002/diff/120001/build/android/resource... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/120001/build/android/resource... build/android/resource_sizes.py:71: _, output = cmd_helper.GetCmdStatusAndOutput(['nm', so_path]) On 2015/11/18 09:55:41, perezju wrote: > if you're going to ignore the exit status use GetCmdOutput Done.
On 2015/11/18 15:05:01, rnephew1 wrote: > https://codereview.chromium.org/1445633002/diff/120001/build/android/resource... > File build/android/resource_sizes.py (right): > > https://codereview.chromium.org/1445633002/diff/120001/build/android/resource... > build/android/resource_sizes.py:71: _, output = > cmd_helper.GetCmdStatusAndOutput(['nm', so_path]) > On 2015/11/18 09:55:41, perezju wrote: > > if you're going to ignore the exit status use GetCmdOutput > > Done. Ping.
lgtm w/ nits https://codereview.chromium.org/1445633002/diff/140001/build/android/resource... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/140001/build/android/resource... build/android/resource_sizes.py:206: for i in apk.infolist(): nit: for i in (x for x in apk.infolist() if IsPakFileName(x.filename)): and nix the if/continue block below https://codereview.chromium.org/1445633002/diff/140001/build/android/resource... build/android/resource_sizes.py:268: i in resource_id_name_map and resource_id_name_map[i] or i, nit: this can just be resource_id_name_map.get(i, i) https://codereview.chromium.org/1445633002/diff/140001/build/android/resource... build/android/resource_sizes.py:385: options.output_dir, 'results-chart.json'), 'w') as json_file: nit: change this indentation. as is, it looks like options.output_dir is another parameter for open(), which isn't the case.
https://codereview.chromium.org/1445633002/diff/140001/build/android/resource... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/140001/build/android/resource... build/android/resource_sizes.py:206: for i in apk.infolist(): On 2015/12/17 17:08:14, jbudorick wrote: > nit: > > for i in (x for x in apk.infolist() if IsPakFileName(x.filename)): > > and nix the if/continue block below Done. https://codereview.chromium.org/1445633002/diff/140001/build/android/resource... build/android/resource_sizes.py:268: i in resource_id_name_map and resource_id_name_map[i] or i, On 2015/12/17 17:08:14, jbudorick wrote: > nit: this can just be > > resource_id_name_map.get(i, i) Done. https://codereview.chromium.org/1445633002/diff/140001/build/android/resource... build/android/resource_sizes.py:385: options.output_dir, 'results-chart.json'), 'w') as json_file: On 2015/12/17 17:08:14, jbudorick wrote: > nit: change this indentation. as is, it looks like options.output_dir is another > parameter for open(), which isn't the case. Done.
The CQ bit was checked by rnephew@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from agrieve@chromium.org, perezju@chromium.org, jbudorick@chromium.org Link to the patchset: https://codereview.chromium.org/1445633002/#ps180001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1445633002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1445633002/180001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by rnephew@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1445633002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1445633002/180001
Message was sent while issue was closed.
Committed patchset #10 (id:180001)
Message was sent while issue was closed.
Description was changed from ========== [Android] Upstream resource_sizes and check_static_initializers and add chartjson support BUG=546012 ========== to ========== [Android] Upstream resource_sizes and check_static_initializers and add chartjson support BUG=546012 Committed: https://crrev.com/f10ebab18d15241a1bd7c0b790a9256e4cafa623 Cr-Commit-Position: refs/heads/master@{#365933} ==========
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/f10ebab18d15241a1bd7c0b790a9256e4cafa623 Cr-Commit-Position: refs/heads/master@{#365933}
Message was sent while issue was closed.
thakis@chromium.org changed reviewers: + thakis@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/1445633002/diff/180001/build/android/resource... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/180001/build/android/resource... build/android/resource_sizes.py:52: 'isolate.cc', why is this completely different from how static initializer tracking is done for the desktop platforms (sizes.py in the build repo)?
Message was sent while issue was closed.
https://codereview.chromium.org/1445633002/diff/180001/build/android/resource... File build/android/resource_sizes.py (right): https://codereview.chromium.org/1445633002/diff/180001/build/android/resource... build/android/resource_sizes.py:52: 'isolate.cc', On 2015/12/17 23:16:45, Nico (office move Wed Thu Fri) wrote: > why is this completely different from how static initializer tracking is done > for the desktop platforms (sizes.py in the build repo)? I didn't write this, I just moved it from downstream clank/build/resource_sizes.py and clank/build/clank_check_static_initializers.py to upstream. Feng made the last few changes to check_static_initializers, but he is no longer with google. Looking at git blame it looks like pliard@ is responsible for the bulk of the static initializer work; but is no longer on Chrome.
Message was sent while issue was closed.
Well, part of upstreaming is to make things not look out of place upstream, no?
Message was sent while issue was closed.
On 2015/12/18 18:40:15, Nico wrote: > Well, part of upstreaming is to make things not look out of place upstream, no? I was looking at moving the implementation from size.py to resource_sizes.py so that they would both use the same implemenation. I noticed something weird about size.py though: https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20ARM... <output of run> # Static initializers in src/out/Release/cronet_sample_apk/libs/arm64-v8a/libcronet.so: # HINT: To get this list, run tools/linux/dump-static-initializers.py # HINT: diff against the log from the last run to see what changed # Found 0 static initializers in 0 files. RESULT cronet_sample_apk_libs_arm64-v8a_libcronet.so: cronet_sample_apk/libs/arm64-v8a/libcronet.so= 4564376 bytes .... RESULT cronet_sample_apk_libs_arm64-v8a_libcronet.so-si: initializers= 2 files </output of run> The output from dump-static-initializers.py is saying that it found 0 static initializes, but is claiming that there are 2 static initializes. I think there might be something wrong with the size.py implementation. Does anyone on this know someone to contact about this? I am not very knowledgeable in this area.
Message was sent while issue was closed.
Thanks for taking a look! sizes.py is here: https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/ch... It uses `readelf -SW` and some output munging (see link) to come up with the number of static initializers. If there are more than 0, it calls https://code.google.com/p/chromium/codesearch#chromium/src/tools/linux/dump-s... to print the list of initializers. It seems like the latter script doesn't handle libcronet.so on arm64_v8a correctly (this is all fairly implementation-specific code; maybe it's easy to fix). You can try running src/tools/linux/dump-static-initializers.py locally to investigate.
Message was sent while issue was closed.
On 2016/01/28 19:46:05, Nico wrote: > Thanks for taking a look! > > sizes.py is here: > https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/ch... > > It uses `readelf -SW` and some output munging (see link) to come up with the > number of static initializers. If there are more than 0, it calls > https://code.google.com/p/chromium/codesearch#chromium/src/tools/linux/dump-s... > to print the list of initializers. It seems like the latter script doesn't > handle libcronet.so on arm64_v8a correctly (this is all fairly > implementation-specific code; maybe it's easy to fix). You can try running > src/tools/linux/dump-static-initializers.py locally to investigate. I ran it locally and I get the same results that are seen on the bot; but it also says: nm: ../../out/Debug/cronet_sample_apk/libs/armeabi-v7a/libcronet.so: no symbols I also ran the size method for getting SI on the android .so files and got this: resource_sizes method: *RESULT StaticInitializersCount: count= 0 count size.py method: *RESULT StaticInitializersCount: count= 32 count and dump-static-initializers.py returns: <snip, just more objdump errors> # scheduler.cc: (empty initializer list) objdump: can't disassemble for architecture UNKNOWN! # site_engagement_service.cc: (empty initializer list) objdump: can't disassemble for architecture UNKNOWN! # synchronous_compositor_impl.cc: (empty initializer list) objdump: can't disassemble for architecture UNKNOWN! # thumbnail_cache.cc: (empty initializer list) # Found 0 static initializers in 31 files. Either the current method is missing a lot of static initializers, or the size.py version is reporting too many.
Message was sent while issue was closed.
"objdump: can't disassemble for architecture UNKNOWN!" is probably the problem, right? That objdump probably can't disassemble arm code, so then dump-static-initializers.py can't work for arm binaries
Message was sent while issue was closed.
On 2016/01/28 20:02:36, Nico wrote: > "objdump: can't disassemble for architecture UNKNOWN!" is probably the problem, > right? That objdump probably can't disassemble arm code, so then > dump-static-initializers.py can't work for arm binaries Thats my thinking. I'm guessing the new SI count of 32 is correct. I'm going to make a CL with these changes so we can switch the discussion to there.
Message was sent while issue was closed.
On 2016/01/28 20:04:13, rnephew1 wrote: > On 2016/01/28 20:02:36, Nico wrote: > > "objdump: can't disassemble for architecture UNKNOWN!" is probably the > problem, > > right? That objdump probably can't disassemble arm code, so then > > dump-static-initializers.py can't work for arm binaries > > Thats my thinking. I'm guessing the new SI count of 32 is correct. I'm going to > make a CL with these changes so we can switch the discussion to there. Thanks for the follow up Randy! |
