From owner-svn-src-head@freebsd.org Wed Oct 31 19:55:15 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id CD4D710E3F24 for ; Wed, 31 Oct 2018 19:55:15 +0000 (UTC) (envelope-from arichardson.kde@gmail.com) Received: from mail-yb1-f174.google.com (mail-yb1-f174.google.com [209.85.219.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 67A1A86546 for ; Wed, 31 Oct 2018 19:55:15 +0000 (UTC) (envelope-from arichardson.kde@gmail.com) Received: by mail-yb1-f174.google.com with SMTP id g9-v6so7166747ybh.7 for ; Wed, 31 Oct 2018 12:55:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=baJafy33D8zKI9xfY5R5K6FUwdn1z5KvJvkDrRXXQgE=; b=HrhLf4tWZI6NuW6P1Mk3dX9cjAM5Uu3Sw8U3p7vPjGeV4NsUrudoP8livF2kEv4JbK 5RLjKkb9SRZqRtRC1vG0D5+Ke1i72g1MvFh17GclyRlV/w2hFH6KCjizQzUR1x3BY41r mY9PLZIlpp/3dihHxOOEhpuC1bSeGFG+OUIGcAytwIVVKeRU54Zp4O8CeCx8/RwiR+O2 KcspVapg1L+OZizBW8UE5fLCbnw7H3ivHGvS5zbUDfJwBkNvL3GiYO3guS+zfzCwOWoL eBWMHoXvieNxfyY0aqdQUOK5MUGbKKx9xi089f/9xVEydzy7M6smCGjaBLy9W6JjWzOd 75/w== X-Gm-Message-State: AGRZ1gJaCJd8Uzz3OKvTbrDzdMoQwSps8Qfdaon/tR6jNTiq8FRSHam7 PdAMelsu4h1fBXcUre2p58LzIKS3nIw= X-Google-Smtp-Source: AJdET5e1akfezK67DSKYiOd+upLCLmYWemV0J9nEGpuu6pO84ErE0EpnpzZ7DCyqf++WWqPSw6zDzg== X-Received: by 2002:a25:69cd:: with SMTP id e196-v6mr4301714ybc.439.1541012047230; Wed, 31 Oct 2018 11:54:07 -0700 (PDT) Received: from mail-yb1-f176.google.com (mail-yb1-f176.google.com. [209.85.219.176]) by smtp.gmail.com with ESMTPSA id j2-v6sm7113085ywe.92.2018.10.31.11.54.06 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 31 Oct 2018 11:54:07 -0700 (PDT) Received: by mail-yb1-f176.google.com with SMTP id d18-v6so7092578yba.4 for ; Wed, 31 Oct 2018 11:54:06 -0700 (PDT) X-Received: by 2002:a25:2d16:: with SMTP id t22-v6mr4299285ybt.85.1541012046677; Wed, 31 Oct 2018 11:54:06 -0700 (PDT) MIME-Version: 1.0 References: <7DC6D9C4-C153-4BCE-851C-22C890AB0D73@yahoo.com> <2D7C5FC6-CA8D-4AAB-BD64-CC883531F737@yahoo.com> In-Reply-To: From: Alexander Richardson Date: Wed, 31 Oct 2018 18:53:55 +0000 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: svn commit: r339876 - head/libexec/rtld-elf To: marklmi26-fbsd@yahoo.com Cc: svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Oct 2018 19:55:16 -0000 On Wed, 31 Oct 2018 at 18:38, Mark Millard wrote: > > On 2018-Oct-30, at 3:23 PM, Alexander Richardson wrote: > > > . . . > > Before this change obj->textsize was always set as the end of > > PT_LOAD[0]. Now it will contain everything up to the end of the last > > PT_LOAD with execute permissions. In the binary you dumped this is > > PT_LOAD[0] both before and after the patch so there is no change in > > behaviour. The .got and .plt we not included in textsize before this > > patch either. Therefore the only way I can see this patch breaking > > anything is if the PHDRS of one of the loaded libraries causes a > > change in behaviour. > > A fair night's sleep helps. > > Not only did I read something into your description that was > not there, I looked at powerpc64 when it turns out that > only 32-bit powerpc shows the problem. (I asked.) > > I'm also told reverting just those changes fixes 32-bit > powerpc context. > > So I'm using an older 32-bit powerpc context to look at: > > https://artifact.ci.freebsd.org/snapshot/head/r339870/powerpc/powerpc/base*.txz > > materials now. -r339870 is the first 32-bit powerpc build > available there after the changes were made. > > So in a /339870/ area from expanding the .txz's > inside it I get: > > # elfdump -p ./bin/ls | less > > program header: > > entry: 0 > p_type: PT_PHDR > p_offset: 52 > p_vaddr: 0x1800034 > p_paddr: 0x1800034 > p_filesz: 224 > p_memsz: 224 > p_flags: PF_X|PF_R > p_align: 4 > > entry: 1 > p_type: PT_INTERP > p_offset: 276 > p_vaddr: 0x1800114 > p_paddr: 0x1800114 > p_filesz: 21 > p_memsz: 21 > p_flags: PF_R > p_align: 1 > > entry: 2 > p_type: PT_LOAD > p_offset: 0 > p_vaddr: 0x1800000 > p_paddr: 0x1800000 > p_filesz: 34112 > p_memsz: 34112 > p_flags: PF_X|PF_R > p_align: 65536 > > entry: 3 > p_type: PT_LOAD > p_offset: 34112 > p_vaddr: 0x1818540 > p_paddr: 0x1818540 > p_filesz: 316 > p_memsz: 1752 > p_flags: PF_X|PF_W|PF_R > p_align: 65536 > > entry: 4 > p_type: PT_DYNAMIC > p_offset: 34132 > p_vaddr: 0x1818554 > p_paddr: 0x1818554 > p_filesz: 216 > p_memsz: 216 > p_flags: PF_W|PF_R > p_align: 4 > > entry: 5 > p_type: PT_NOTE > p_offset: 300 > p_vaddr: 0x180012c > p_paddr: 0x180012c > p_filesz: 48 > p_memsz: 48 > p_flags: PF_R > p_align: 4 > > entry: 6 > p_type: PT_LOAD > p_offset: 0 > p_vaddr: 0 > p_paddr: 0 > p_filesz: 0 > p_memsz: 0 > p_flags: PF_W|PF_R > p_align: 4 > > I note some things unique to this compared > to powerpc64 and to what the old code would > do for what is unique: > > There are 2 PT_LOADS with PF_X and there are a > bunch of pages between that are not covered by > any entry. This is from p_align being 65536 from > what I can tell. > > entry 2: > 0x1800000+34112==0x1808540 for the ending of the read-only PF_X area. > entry 3: > 0x1818540 for the beginning of the writable PF_X area. > 0x0010000 differences: 65536 Bytes. > > What is the handling of the page range that is > not described in any entry? Is > __syncicache(obj->mapbase, obj->textsize) > spanning the hole valid? Previously the > hole was not spanned as I understand. > > > Libraries also have the hole between the > PT_LOAD with PF_X ranges. Using /lib/libc.so > as an example: > > # elfdump -p ./lib/libc.so.7 | less > > program header: > > entry: 0 > p_type: PT_LOAD > p_offset: 0 > p_vaddr: 0 > p_paddr: 0 > p_filesz: 1746472 > p_memsz: 1746472 > p_flags: PF_X|PF_R > p_align: 65536 > > entry: 1 > p_type: PT_LOAD > p_offset: 1746480 > p_vaddr: 0x1ba630 > p_paddr: 0x1ba630 > p_filesz: 44188 > p_memsz: 201536 > p_flags: PF_X|PF_W|PF_R > p_align: 65536 > > entry: 2 > p_type: PT_DYNAMIC > p_offset: 1762600 > p_vaddr: 0x1be528 > p_paddr: 0x1be528 > p_filesz: 192 > p_memsz: 192 > p_flags: PF_W|PF_R > p_align: 4 > > entry: 3 > p_type: PT_TLS > p_offset: 1746480 > p_vaddr: 0x1ba630 > p_paddr: 0x1ba630 > p_filesz: 2832 > p_memsz: 2860 > p_flags: PF_R > p_align: 16 > > entry: 4 > p_type: PT_NULL > p_offset: 1745288 > p_vaddr: 0x1aa188 > p_paddr: 0x1aa188 > p_filesz: 236 > p_memsz: 236 > p_flags: PF_R > p_align: 4 > > entry: 5 > p_type: PT_LOAD > p_offset: 0 > p_vaddr: 0 > p_paddr: 0 > p_filesz: 0 > p_memsz: 0 > p_flags: PF_W|PF_R > p_align: 4 > > 1746472==0x1AA628 for the ending of the readonly code. > 0x1ba630 for the beginning of the writable code. > 0x10008 difference: 655544 bytes. > > > I doubt the following would contribute but I > note them: > > PT_PHDR for ./bin/ls has PF_X indicated. > > Various other entries overlap with the > PT_LOAD's that have PF_X. > This seems to indicate that it is not possible to for __syncicache() to span unmapped ranges. It seems to me like the current behaviour for flushing the icache was making some assumptions that do not actually hold and my change exposed them. Apparently we have some DSOs that have multiple PF_X PT_LOAD segments (which should probably be included in the __syncicache() call) but currently only the value of the first PT_LOAD is included. This will work correctly for most cases since PT_LOAD[0] usually contains .text but will break once we start using LLD since LLD sometimes places a read-only segment first. I'm not sure what the correct solution is here. Clearly, we should be flushing the icache for all executable segments and just PT_LOAD[0] (which may not even be executable). If it's not possible to use a single contiguous range we might have to store a list. Alex