Skip to content
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

TIOCSWINSZ hack produces ioctl sign-extension warning in FreeBSD system log #39

Closed
emaste opened this issue Feb 6, 2014 · 13 comments
Closed

Comments

@emaste
Copy link
Contributor

emaste commented Feb 6, 2014

There's a hack in pexpect/__init__.py to work around some buggy platforms:

    # Check for buggy platforms. Some Python versions on some platforms     
    # (notably OSF1 Alpha and RedHat 7.1) truncate the value for            
    # termios.TIOCSWINSZ. It is not clear why this happens.                 
    # These platforms don't seem to handle the signed int very well;        
    # yet other platforms like OpenBSD have a large negative value for      
    # TIOCSWINSZ and they don't have a truncate problem.                    
    # Newer versions of Linux have totally different values for TIOCSWINSZ. 
    # Note that this fix is a hack.                                         
    TIOCSWINSZ = getattr(termios, 'TIOCSWINSZ', -2146929561)               
    if TIOCSWINSZ == 2148037735L: # L is not required in Python >= 2.2.    
        TIOCSWINSZ = -2146929561 # Same bits, but with sign.               

-2146929561 is 0xffffffff80087467 and passing this sign-extended value rather than the desired 0x80087467 produces a warning message in the system console on FreeBSD:

Feb 5 17:19:11 feynman kernel: WARNING pid 11323 (python2.7): ioctl sign-extension ioctl ffffffff80087467

This has shown up in a few places, e.g.:
http://www.freebsd.org/cgi/query-pr.cgi?pr=152770
http://llvm.org/bugs/show_bug.cgi?id=18749

@takluyver
Copy link
Member

Thanks for reporting this. So, to check that I understand correctly:

  • Without this hack, the setwinsize method will fail on some systems. At this point, it's probably only really ancient systems that are affected, but it's difficult to be sure.
  • With this hack, calling the method emits a warning in the system log but still succeeds, on BSD systems, and presumably also OS X.

Does anyone have any more information on what platforms need this workaround? I can certainly imagine that it's no longer necessary, but I'm reluctant to drop it if it might cause actual issues again (and real problems trump warnings). Specifically, I'm concerned about 32 bit systems, and enterprisey systems which go years without upgrading the core system.

@emaste
Copy link
Contributor Author

emaste commented Feb 6, 2014

Your with/without scenarios match my understanding of this issue. I have no insight into which platforms are affected though, and can understand the reluctance to dropping it.

What about whitelisting platforms known to not need the workaround - a la

if (not sys.platform.startswith("freebsd")):
    workaround hack

@takluyver
Copy link
Member

Hmm, we could do that, but it's really a question of platform+version, and that opens up a big can of worms.

If we can check a decent variety of current platforms, I think the best option is to drop the hack - if there's some niche platform that still has the issue, they can either patch it locally or work out a fix that doesn't affect other platforms.

I'd like to confirm that this isn't needed on as many combinations as possible of:

  • Kernels: Linux, BSD, Darwin, Solaris? Other unixes?
  • Architectures: x64, x86, ARM? PPC?
  • Old versions of kernels

So far, I've checked on Linux x64 and Mac x64, and the bug reports are from BSD, presumably x64.

@emaste
Copy link
Contributor Author

emaste commented Feb 11, 2014

The comment mentions "OSF1 Alpha and RedHat 7.1" as examples of affected systems.

I searched only for FreeBSD bug reports; I'm not sure if other BSDs have the same warning or not. Presumably the workaround is only applicable to 64-bit systems.

@takluyver
Copy link
Member

I'm guessing that that means Red Hat Linux, not RHEL - so 7.1 was released way back in 2001. From what I can find on OSF/1, that's even older. But I've no idea how long the bug may have persisted, and if it might still be out there in Linux versions that big enterprises still run things on.

@tomspur
Copy link
Contributor

tomspur commented Feb 13, 2014

This workaround seems to depend on the operation system and not the architecture (With a recent Fedora version armv7a, aarch64 and x86-64 all contain the same value of 21524 - thanks @hrw for confirming that on #fedora-arm).

Isn't it quite save to assume, that if enterprises still run such a old Linux distribution, they most likely don't want to update pexpect anyway and stay on the old version?

@takluyver
Copy link
Member

Thanks @tomspur , that's the same value I get on Debian on x86-64.

If enterprises are still running something from 2001, I certainly don't mind if they can't update pexpect. I was thinking of more recent distros that might still be in use, like RHEL 5, first released in 2007. But then, RHEL 5 had Python 2.4, and Pexpect already doesn't support that.

I think we should get rid of this workaround and wait to see if anyone complains. @emaste , do you want to make a pull request? Drop a comment in where it was to note that it was removed.

@emaste
Copy link
Contributor Author

emaste commented Feb 13, 2014

The workaround is architecture dependent in that it deals with sign extension on 64-bit systems, but only affects platforms where TIOCSWINSZ has a value > 0x80000000.

For example, on FreeBSD:

hex(getattr(termios, 'TIOCSWINSZ'))
'0x80087467'

This is the magic value that the workaround is "helpfully" updating.

    if TIOCSWINSZ == 2148037735:                                           
        # Same bits, but with sign.                                        
        TIOCSWINSZ = -2146929561                                           

The issue is that it's the same lower 32 bits, but the 2nd parameter to ioctl is an unsigned long, so the replacement value -2146929561 ends up as 0xffffffff80087467, not 0x80087467, which is what produces the warning from the FreeBSD kernel.

@tomspur
Copy link
Contributor

tomspur commented Feb 13, 2014

@takluyver: CentOS 5 at least also has the same value in TIOCSWINSZ, so I'm guessing RHEL 5 should report the same. I'm also having access to Debian 7 (wheezy), SLES 10 (both 64 bit), which all have the same value... It would be nice to get that value from an android device or so, but other than that, I don't know which system could still need that workaround...

@takluyver
Copy link
Member

I'm just setting up Python on my Android tablet to check ;-)

@tomspur
Copy link
Contributor

tomspur commented Feb 13, 2014

👍 :)

@takluyver
Copy link
Member

It's 21524 in Android 4.4 as well.

@takluyver
Copy link
Member

Fixed by PR #40.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants