Bug #10469
GCC's -faggressive-loop-optimizations is too aggressive
Start date:
2019-02-27
Due date:
% Done:
100%
Estimated time:
Difficulty:
Medium
Tags:
Description
Based on experience in OS-7511 with GCC optimizations eliding undefined but de facto correct code, a full build was performed with -fno-aggressive-loop-optimizations and then functions were compared to determine changes in text size. First, it should be said that most functions were not affected at all; of the 782027 functions, there were text size differences in only 2286 of them. Of these, text size increased in about half of them (1026). In investigating these, they seem to primarily be due to different register selection, often because the presence of the optimization seems to cause loop counters (and address calculation) to be replaced with addresses and address bounds.
Here are the functions for which -faggressive-loop-optimizations resulted in less program text and fewer backwards branches, ordered by fraction of text eliminated:
#backwards branches, -faggressive-loop-optimizations ------------------------+
#backwards branches, -fno-aggressive-loop-optimizations ----------------+ |
#instructions, -faggressive-loop-optimizations --------------------+ | |
#instructions, -fno-aggressive-loop-optimizations ----------+ | | |
| | | |
V V V V
MOD`FUNC #INST O#INST #BB O#BB
usr/lib/libpool.so.1`pool_resource_type_list 116 66 2 0
kernel/drv/amd64/xge`xgell_tx_close 122 71 2 0
platform/i86pc/kernel/drv/amd64/dr`dr_bypass_device 82 50 1 0
usr/lib/libadutils.so.1`check_for_binary_attrs 80 51 1 0
kernel/drv/amd64/qede`qede_free_tx_bd_ring 122 80 1 0
usr/sbin/i86/zdb`get_metaslab_refcount 223 147 7 6
kernel/drv/amd64/qede`qede_alloc_io_structs 166 112 2 1
usr/lib/amd64/libstandsaveargs.so`has_saved_fp 220 151 4 2
usr/lib/amd64/libsaveargs.so.1`has_saved_fp 220 151 4 2
kernel/misc/amd64/kmdbmod`has_saved_fp 220 151 4 2
kernel/drv/amd64/qede`ecore_int_disable_post_isr_release 67 46 1 0
usr/kernel/fs/amd64/lx_proc`lxpr_skip_mntopt 91 63 1 0
usr/lib/libzpool.so.1`vdev_root_core_tvds 172 121 5 4
usr/lib/amd64/gss/mech_krb5.so.1`rem_com_err_hook 187 135 4 3
usr/lib/smbsrv/amd64/libfksmbsrv.so.1`smb2_supported_version 95 71 1 0
usr/lib/amd64/libvarpd.so.1`libvarpd_prop_set 194 146 2 1
usr/lib/ndmp/ndmpd`ndmpd_make_exc_list 78 59 1 0
kernel/fs/amd64/zfs`dnode_needs_remap 252 191 3 1
kernel/drv/amd64/smrt`smrt_interrupts_free 101 77 1 0
usr/lib/libzpool.so.1`dnode_needs_remap 325 250 4 3
usr/lib/libzpool.so.1`vdev_count_leaves_impl 101 79 3 2
kernel/fs/amd64/zfs`dmu_objset_write_ready 175 137 1 0
usr/lib/amd64/libzpool.so.1`dnode_needs_remap 309 245 4 2
usr/lib/amd64/libadutils.so.1`check_for_binary_attrs 114 92 1 0
kernel/drv/amd64/qede`qede_fastpath_config 447 364 2 1
usr/kernel/drv/amd64/smbsrv`smb2_supported_version 87 71 1 0
usr/lib/libzpool.so.1`dmu_object_info_from_dnode 405 339 3 1
kernel/drv/amd64/emlxs`emlxs_fcoe_fcftab_fcfi_online_action 618 518 5 3
usr/sbin/i86/zdb`get_dtl_refcount 140 118 3 2
usr/lib/amd64/libzfs_jni.so.1`zjni_get_default_property 283 241 4 3
usr/lib/mdb/kvm/amd64/fcp.so`targets_walk_i 212 182 4 3
kernel/kmdb/amd64/fcp`targets_walk_i 212 182 4 3
kernel/drv/amd64/qede`qede_init_locks 341 293 2 1
kernel/drv/amd64/bnxe`elink_avoid_link_flap 551 474 7 6
kernel/fs/amd64/zfs`dmu_object_info_from_dnode 323 280 2 1
usr/xpg6/bin/view`dhl_doit 242 210 4 2
usr/xpg6/bin/vi`dhl_doit 242 210 4 2
usr/xpg6/bin/vedit`dhl_doit 242 210 4 2
usr/xpg6/bin/expr`dhl_doit 242 210 4 2
usr/xpg6/bin/ex`dhl_doit 242 210 4 2
usr/xpg6/bin/edit`dhl_doit 242 210 4 2
usr/xpg6/bin/ed`dhl_doit 242 210 4 2
usr/xpg4/bin/view`dhl_doit 242 210 4 2
usr/xpg4/bin/vi`dhl_doit 242 210 4 2
usr/xpg4/bin/vedit`dhl_doit 242 210 4 2
usr/xpg4/bin/nl`dhl_doit 242 210 4 2
usr/xpg4/bin/expr`dhl_doit 242 210 4 2
usr/xpg4/bin/ex`dhl_doit 242 210 4 2
usr/xpg4/bin/edit`dhl_doit 242 210 4 2
usr/xpg4/bin/ed`dhl_doit 242 210 4 2
usr/bin/red`dhl_doit 242 210 4 2
usr/bin/nl`dhl_doit 242 210 4 2
usr/bin/expr`dhl_doit 242 210 4 2
usr/bin/ed`dhl_doit 242 210 4 2
kernel/drv/amd64/emlxs`emlxs_fcoe_fcftab_offline_timer 186 162 1 0
usr/lib/libzpool.so.1`dmu_objset_write_ready 397 346 5 3
lib/amd64/libnsl.so.1`getflag 141 123 4 3
lib/amd64/libadm.so.1`puttext 1833 1601 35 31
usr/sbin/iscsiadm`usage 270 237 7 4
usr/lib/amd64/libzpool.so.1`dmu_object_info_from_dnode 355 312 2 1
lib/libdhcputil.so.1`category_to_code 89 79 2 1
lib/amd64/libcurses.so.1`waddwchnstr 721 641 12 9
kernel/drv/amd64/nxge`nxge_update_rxdma_grp_properties 1440 1284 28 27
kernel/drv/amd64/nxge`nxge_cfg_verify_set 655 585 17 15
kernel/drv/amd64/qede`qede_destroy_locks 200 179 2 1
lib/libc.so.1`door_xcreate_n 1271 1139 26 20
kernel/drv/amd64/qede`qede_fastpath_start_queues 824 739 6 3
usr/lib/libc/libc_hwcap3.so.1`door_xcreate_n 1245 1121 23 20
usr/lib/libc/libc_hwcap2.so.1`door_xcreate_n 1245 1121 23 20
usr/lib/libc/libc_hwcap1.so.1`door_xcreate_n 1245 1121 23 20
usr/lib/amd64/gss/mech_krb5.so.1`krb5_c_keyed_checksum_types 338 305 7 3
usr/lib/security/amd64/pkcs11_kernel.so.1`kernel_get_func_list 430 390 6 4
kernel/drv/amd64/mc-amd`iaddr_to_bank 188 171 2 1
usr/lib/amd64/libzpool.so.1`dmu_objset_write_ready 354 322 3 2
lib/libnvpair.so.1`nvt_resize 222 202 4 3
lib/libcurses.so.1`waddwchnstr 713 649 18 16
kernel/drv/amd64/emlxs`emlxs_fcoe_fcftab_fcfi_offline_action 456 416 3 2
platform/i86xpv/kernel/amd64/unix`page_ctrs_cleanup 195 178 2 1
platform/i86pc/kernel/amd64/unix`page_ctrs_cleanup 195 178 2 1
usr/sbin/i86/zdb`dump_bpobj_subobjs 355 325 5 4
kernel/drv/amd64/qede`ecore_hw_timers_stop_all 98 90 1 0
lib/libnsl.so.1`getflag 89 82 5 2
usr/lib/amd64/gss/mech_krb5.so.1`set_com_err_hook 118 109 1 0
kernel/drv/amd64/emlxs`emlxs_parse_prog_types 1633 1509 47 46
lib/amd64/libdhcputil.so.1`category_to_code 108 100 2 1
lib/amd64/libzfs.so.1`zfeature_register 222 206 2 1
usr/lib/krb5/libkadm5srv.so.1`kadm5_free_key_data 127 118 2 1
usr/lib/krb5/libkadm5clnt.so.1`kadm5_free_key_data 127 118 2 1
kernel/drv/amd64/ixgbe`ixgbe_chip_start 345 321 5 4
kernel/drv/amd64/qede`qede_kstat_update 1078 1004 4 3
kernel/fs/amd64/zfs`zfeature_register 238 222 2 1
usr/kernel/fs/amd64/lx_proc`lxpr_open 286 267 5 3
kernel/drv/amd64/xge`xgell_tx_open 109 102 1 0
kernel/fs/amd64/zfs`vdev_raidz_reconstruct 344 322 14 13
kernel/drv/amd64/nxge`nxge_update_txdma_properties 1478 1385 27 24
kernel/drv/amd64/nxge`nxge_update_rxdma_properties 1478 1385 27 24
kernel/drv/amd64/qede`qede_save_fp_dma_handles 479 449 4 3
kernel/drv/amd64/qede`qede_free_tx_ring_phys 130 122 1 0
usr/bin/amd64/nm`readsyms 776 729 15 14
kernel/strmod/amd64/ip`rule_prefix 117 110 1 0
kernel/drv/amd64/ip`rule_prefix 117 110 1 0
usr/bin/geniconvtbl`map_table_hash 1720 1619 39 38
kernel/fs/amd64/udfs`ud_read_sparing_tbls 327 308 4 2
usr/sbin/isns`setup_ref_lcp 437 412 7 5
platform/i86xpv/kernel/amd64/unix`page_ctrs_sz 581 548 9 8
platform/i86pc/kernel/amd64/unix`page_ctrs_sz 581 548 9 8
usr/bin/ftp`mypopen 480 453 11 9
usr/lib/libzpool.so.1`vdev_compact_children 267 252 6 4
usr/lib/amd64/libtecla.so.1`gl_start_newline 220 208 3 2
lib/amd64/libnsl.so.1`expect_str 303 287 3 1
usr/kernel/drv/amd64/ipf`ippr_rpcb_modv4 537 509 4 2
kernel/misc/amd64/usba`usb_pipe_isoc_xfer 422 400 6 5
usr/lib/amd64/gss/mech_krb5.so.1`reset_com_err_hook 117 111 1 0
usr/lib/gss/mech_krb5.so.1`krb5int_get_plugin_filenames 326 310 7 6
usr/lib/amd64/libldap.so.5`ldif_base64_encode_internal 502 478 12 11
usr/sbin/isnsadm`cvt_enumerate_rsp_to_get_req 1517 1445 31 30
usr/lib/amd64/libast.so.1`_ast_ftwalk 1105 1057 27 24
usr/lib/nss_ldap.so.1`add_netgroup_name 236 226 7 5
usr/lib/libsun_ima.so.1`isAuthMethodValid 98 94 2 1
kernel/misc/amd64/nfssrv`nfslog_write_record 735 706 8 5
usr/lib/libdtrace.so.1`dt_fprintas 128 123 3 2
usr/lib/ipf/amd64/ipftest`ippr_rpcb_modv4 449 432 5 3
lib/libresolv.so.2`res_nsend 2223 2139 32 31
usr/sbin/snoop`concat_args 213 205 6 5
kernel/drv/amd64/bnxe`BnxeHwReqPhyFlowSettings 493 475 5 4
usr/lib/libzpool.so.1`raidz_parity_verify 367 354 6 5
usr/sbin/ipqosconf`get_originator_nm 57 55 3 1
usr/ucb/groups`main 286 276 4 3
usr/bin/amd64/savecore`BZ2_hbMakeCodeLengths 1375 1327 26 25
kernel/amd64/genunix`BZ2_hbMakeCodeLengths 1375 1327 26 25
platform/i86xpv/kernel/drv/amd64/xsvc`xsvc_devmap 774 747 8 7
usr/lib/libzfs_jni.so.1`zjni_get_default_property 324 313 10 9
platform/i86pc/kernel/drv/amd64/xsvc`xsvc_devmap 750 725 8 7
usr/lib/amd64/libsldap.so.1`__ns_ldap_trydoorcall_getfd 496 480 8 6
usr/bin/rdist`docmdargs 409 396 8 7
usr/kernel/fs/amd64/lx_proc`lxpr_doaccess 519 503 14 13
lib/amd64/libc.so.1`__k_gconvert 801 777 20 18
kernel/drv/amd64/yge`yge_resume 541 525 7 6
platform/i86xpv/kernel/amd64/unix`hment_mapcnt 273 265 6 4
platform/i86pc/kernel/amd64/unix`hment_mapcnt 273 265 6 4
kernel/drv/amd64/qede`qede_fastpath_stop_queues 567 551 7 6
kernel/drv/amd64/si3124`si_watchdog_handler 538 523 10 8
usr/lib/amd64/gss/mech_krb5.so.1`prof_hostnames2netaddrs 582 566 13 12
kernel/drv/amd64/rpcib`rib_svc_scq_handler 329 320 5 4
kernel/drv/amd64/rpcib`rib_clnt_scq_handler 329 320 5 4
kernel/misc/amd64/usba`usb_pipe_intr_xfer 597 581 10 9
usr/sbin/iasl`LsFlushListingBuffer 608 592 9 8
kernel/drv/amd64/ural`ural_set_chan 1107 1078 11 10
kernel/misc/amd64/usba`usb_pipe_ctrl_xfer 618 602 11 9
usr/bin/amd64/pflags`lwplook 961 937 15 14
lib/amd64/libresolv.so.2`res_nsend 2119 2071 31 29
kernel/drv/amd64/t4nex`t4_devo_detach 819 801 9 8
usr/platform/i86pc/lib/fm/topo/plugins/x86pi.so`x86pi_bb_topparent 393 385 9 7
usr/bin/tail`lines 1033 1012 28 27
usr/sbin/isnsadm`cmdParse 1880 1842 34 32
usr/bin/lex`acompute 768 753 20 19
kernel/drv/amd64/qede`qede_alloc_tx_ring_phys 362 355 5 0
usr/bin/audiotest`uncompress_wave 1379 1353 15 14
kernel/drv/amd64/dcam1394`dcam_ioctl 850 834 19 17
usr/lib/amd64/libzpool.so.1`vdev_raidz_reconstruct 856 840 23 22
usr/bin/i86/ztest`make_vdev_root 482 473 7 6
kernel/misc/amd64/pcmcia`SSSetSocket 379 372 2 0
usr/sbin/stmfadm`cmdParse 2078 2040 34 32
usr/sbin/sbdadm`cmdParse 2078 2040 34 32
usr/sbin/sasinfo`cmdParse 2078 2040 34 32
usr/sbin/fcinfo`cmdParse 2078 2040 34 32
usr/sbin/fcadm`cmdParse 2078 2040 34 32
usr/demo/comstar/bin/aluaadm`cmdParse 2078 2040 34 32
lib/svc/method/svc-stmf`cmdParse 2078 2040 34 32
lib/svc/method/iscsi-target`cmdParse 2078 2040 34 32
usr/lib/amd64/libzpool.so.1`zfs_blkptr_verify 938 921 19 18
kernel/drv/amd64/bnxe`BnxeHwReqPhyMediumSettings 3087 3032 43 41
usr/lib/amd64/libsldap.so.1`sortServerPref 907 891 30 29
kernel/drv/amd64/yge`yge_detach 455 447 2 1
kernel/misc/amd64/usba`usb_pipe_bulk_xfer 467 459 10 8
usr/lib/libsldap.so.1`verify_value 733 721 18 10
usr/lib/libc/libc_hwcap3.so.1`glob0 1447 1424 66 62
usr/lib/libc/libc_hwcap3.so.1`glob0 1447 1424 66 62
usr/lib/libc/libc_hwcap2.so.1`glob0 1447 1424 66 62
usr/lib/libc/libc_hwcap2.so.1`glob0 1447 1424 66 62
usr/lib/libc/libc_hwcap1.so.1`glob0 1447 1424 66 62
usr/lib/libc/libc_hwcap1.so.1`glob0 1447 1424 66 62
usr/lib/ipf/i86/ipftest`ippr_rpcb_modv4 441 434 5 3
usr/sbin/amd64/lockstat`dprog_addevent 1116 1100 11 10
usr/lib/gss/mech_krb5.so.1`rem_com_err_hook 146 144 5 4
usr/bin/troff`casecs 146 144 3 1
usr/sbin/mpathadm`cmdParse 2585 2550 39 37
usr/bin/split`main 2954 2914 55 54
usr/sbin/iscsiadm`cmdParse 2623 2588 39 37
usr/lib/mdb/kvm/amd64/genunix.so`Pcred_gcore 309 305 3 2
kernel/drv/amd64/bnxe`ecore_config_vlan_mac 391 386 8 5
usr/sbin/ndmpadm`ndmp_kill_sessions 236 233 5 4
lib/amd64/libc.so.1`format_grouped_double 1287 1271 17 16
usr/bin/pg`main 1384 1367 29 28
usr/lib/libzpool.so.1`dnode_sync 2639 2607 44 42
lib/amd64/libtsol.so.2`__call_labeld 1039 1027 13 12
usr/sbin/snoop`detail_attr_bitmap 260 257 6 5
kernel/drv/amd64/vmxnet`Vxn_AllocInitBuffers 626 619 6 5
kernel/drv/amd64/mc-amd`rct_csintlv_bits 359 355 10 9
usr/lib/mdb/kvm/amd64/genunix.so`pfile_callback 1546 1529 34 33
kernel/drv/amd64/yge`yge_attach 1180 1168 18 14
usr/lib/libzpool.so.1`zfeature_register 409 405 6 5
kernel/fs/amd64/zfs`zil_lwb_write_issue 880 872 6 4
usr/lib/amd64/libsun_ima.so.1`setAuthMethods 346 343 4 2
kernel/drv/amd64/ixgbe`ixgbe_set_priv_prop 822 815 23 22
kernel/kmdb/amd64/genunix`stacks 3019 2995 55 51
usr/lib/mdb/proc/amd64/libc.so`stacks 3051 3027 54 50
usr/lib/mdb/kvm/amd64/genunix.so`stacks 3051 3027 54 50
usr/sbin/lofiadm`GetOptimum 5338 5297 76 75
kernel/drv/amd64/bnxe`lm_cmng_init 1429 1419 19 18
usr/lib/amd64/libzpool.so.1`zil_lwb_write_issue 1269 1261 11 9
kernel/kmdb/amd64/nca`nca_io2 1273 1265 14 12
usr/lib/mdb/kvm/amd64/nca.so`nca_io2 1313 1305 14 12
lib/amd64/libnsl.so.1`getipnodebyname 1453 1445 31 29
usr/bin/i86/truss`report 1409 1402 22 21
usr/bin/lex`packtrans 1999 1990 47 44
usr/lib/zones/zonestatd`zsd_refresh_memory 2708 2697 38 37
kernel/kmdb/amd64/genunix`pfile_callback 1275 1270 33 32
kernel/drv/amd64/pcic`pcic_setup_adapter 2602 2592 51 46
usr/bin/amd64/savecore`sendMTFValues 12636 12598 117 116
kernel/amd64/genunix`sendMTFValues 12768 12730 117 116
usr/lib/fm/amd64/libfmd_msg.so.1`fmd_msg_getitem_locked 1036 1033 13 12
usr/lib/zones/amd64/zoneadmd`add_net_for_linkid 1673 1669 20 19
lib/libc.so.1`__lc_collate_load 880 878 14 5
usr/sbin/isnsadm`subUsage 920 918 14 13
usr/sbin/stmfadm`subUsage 944 942 14 13
usr/sbin/sbdadm`subUsage 944 942 14 13
usr/sbin/sasinfo`subUsage 944 942 14 13
usr/sbin/fcinfo`subUsage 944 942 14 13
usr/sbin/fcadm`subUsage 944 942 14 13
usr/demo/comstar/bin/aluaadm`subUsage 944 942 14 13
lib/svc/method/svc-stmf`subUsage 944 942 14 13
lib/svc/method/iscsi-target`subUsage 944 942 14 13
kernel/drv/amd64/nv_sata`ck804_intr_process 1190 1188 21 17
usr/xpg6/bin/ls`pentry 2076 2074 37 36
usr/xpg4/bin/ls`pentry 2076 2074 37 36
usr/bin/ls`pentry 2076 2074 37 36
Of these, many (if not most) represent legitimate optimizations -- instances where the compiler unrolled a loop for which it can use the static size information to determine the number of iterations. For at least one class, though, -faggressive-loop-optimizations resulted in deeply incorrect code: any code iterating over dn_nblkptr in ZFS.
Take, for example, this code in dmu_objset_write_ready:
for (int i = 0; i < dnp->dn_nblkptr; i++)
bp->blk_fill += BP_GET_FILL(&dnp->dn_blkptr[i]);
Here is the generated code for this with -faggressive-loop-optimizations:
dmu_objset_write_ready+0x27: f6 40 74 80 testb $0x80,0x74(%rax)
dmu_objset_write_ready+0x2b: ba 01 00 00 00 movl $0x1,%edx
dmu_objset_write_ready+0x30: 75 07 jne +0x7 <dmu_objset_write_ready+0x39>
dmu_objset_write_ready+0x32: 48 8b 90 98 00 00 movq 0x98(%rax),%rdx
00
dmu_objset_write_ready+0x39: 48 89 53 58 movq %rdx,0x58(%rbx)
Note: no loop! This code should look something more like:
dmu_objset_write_ready+0x27: 48 8d 46 70 leaq 0x70(%rsi),%rax
dmu_objset_write_ready+0x2b: 31 c9 xorl %ecx,%ecx
dmu_objset_write_ready+0x2d: 31 d2 xorl %edx,%edx
dmu_objset_write_ready+0x2f: 49 b8 00 00 00 00 movq $0x8000000000,%r8
80 00 00 00
dmu_objset_write_ready+0x39: 0f 1f 80 00 00 00 nopl 0x0(%rax)
00
dmu_objset_write_ready+0x40: 4c 85 00 testq %r8,(%rax)
dmu_objset_write_ready+0x43: bf 01 00 00 00 movl $0x1,%edi
dmu_objset_write_ready+0x48: 75 04 jne +0x4 <dmu_objset_write_ready+0x4e>
dmu_objset_write_ready+0x4a: 48 8b 78 28 movq 0x28(%rax),%rdi
dmu_objset_write_ready+0x4e: 48 01 f9 addq %rdi,%rcx
dmu_objset_write_ready+0x51: ff c2 incl %edx
dmu_objset_write_ready+0x53: 48 83 e8 80 subq $-0x80,%rax <0xffffffffffffff80>
dmu_objset_write_ready+0x57: 48 89 4b 58 movq %rcx,0x58(%rbx)
dmu_objset_write_ready+0x5b: 0f b6 7e 03 movzbl 0x3(%rsi),%edi
dmu_objset_write_ready+0x5f: 39 d7 cmpl %edx,%edi
dmu_objset_write_ready+0x61: 7f dd jg -0x23 <dmu_objset_write_ready+0x40>
The problem here is that dn_blkptr is used somewhat craftily; it is declared to be an array of size 1 (that is, appearing to be a flexible array member), but in fact is used to index into dn_bonus:
typedef struct dnode_phys {
uint8_t dn_type; /* dmu_object_type_t */
uint8_t dn_indblkshift; /* ln2(indirect block size) */
uint8_t dn_nlevels; /* 1=dn_blkptr->data blocks */
uint8_t dn_nblkptr; /* length of dn_blkptr */
uint8_t dn_bonustype; /* type of data in bonus buffer */
uint8_t dn_checksum; /* ZIO_CHECKSUM type */
uint8_t dn_compress; /* ZIO_COMPRESS type */
uint8_t dn_flags; /* DNODE_FLAG_* */
uint16_t dn_datablkszsec; /* data block size in 512b sectors */
uint16_t dn_bonuslen; /* length of dn_bonus */
uint8_t dn_pad2[4];
/* accounting is protected by dn_dirty_mtx */
uint64_t dn_maxblkid; /* largest allocated block ID */
uint64_t dn_used; /* bytes (or sectors) of disk space */
uint64_t dn_pad3[4];
blkptr_t dn_blkptr[1];
uint8_t dn_bonus[DN_MAX_BONUSLEN - sizeof (blkptr_t)];
blkptr_t dn_spill;
} dnode_phys_t;
The ZoL folks also ran into this ; their fix was to compile with -fno-aggressive-loop-optimizations -- but they ultimately fixed the structure and then re-enabled the optimization . While this structure should definitely be changed to give the compiler a better idea of what's going on, it feels too risky to leave -faggressive-loop-optimizations on (it is the default with -O2). The win it delivers seems far too modest to pay for the risk of potential data corruption induced by incorrect code generation; we should compile with -fno-aggressive-loop-optimizations.
We should disable this dangerous option across illumos.
This bug mirrors Joyent upstream bug OS-7517
Related issues
History
Updated by
Updated by John Levon 5 months ago
- Related to Feature #9996: use GCC 7 as default primary compiler added
Updated by
Updated by Joshua Clulow 5 months ago
Additional comments from original ticket:¶
Bryan Cantrill added a comment - 16/Jan/19 11:42 AM¶
Two additional notes. First, the difference in has_saved_fp is actually due to an array over-read; see OS-7518 (#10470). Second, vdev_raidz_reconstruct (which troublingly appears in the list of potentially suspect functions) appears to have correct code generated with -faggressive-loop-optimizations, and is in that regard apparently a false positive.
John Levon added a comment - 17/Jan/19 2:06 AM¶
Previously we thought that GCC would do this upon 1-sized last-member arrays (not just the dn_blkptr grossness). Was that mistaken, or is that still potentially the case? The reason I ask is that the GCC source in this area has special code for this, implying that if it is doing bad things, it would actually be considered a GCC Bug.
Bryan Cantrill added a comment - 22/Jan/19 9:11 AM¶
It appears that GCC is indeed handling the 1-sized last-member arrays properly (fortunately). The code generated is quite different (which led to the concern that loops were being obviated), but having gone over some of these functions (e.g. vdev_raidz_reconstruct) quite carefully, it seems that the code is correct in this case, even in the presence of -faggressive-loop-optimizations.
Updated by Electric Monk 5 months ago
Updated by - Status changed from New to Closed
- % Done changed from 0 to 100
git commit 92c1a61163ff6a0655b27bd429856e171e7ce5f5
commit 92c1a61163ff6a0655b27bd429856e171e7ce5f5
Author: Bryan Cantrill <bryan@joyent.com>
Date: 2019-03-01T11:11:54.000Z
10468 __ctype_mask[EOF] has been working by accident
10469 GCC's -faggressive-loop-optimizations is too aggressive
10470 array over-read in has_saved_fp()
Reviewed by: Robert Mustacchi <rm@joyent.com>
Reviewed by: John Levon <john.levon@joyent.com>
Reviewed by: Toomas Soome <tsoome@me.com>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Reviewed by: Andy Fiddaman <andy@omniosce.org>
Reviewed by: Gergő Doma <domag02@gmail.com>
Reviewed by: Gary Mills <gary_mills@fastmail.fm>
Approved by: Richard Lowe <richlowe@richlowe.net>